2012/5/18 Jeff Layton <jlayton@xxxxxxxxxx>:
> On Thu, 17 May 2012 19:43:43 +0400
> Pavel Shilovsky <piastry@xxxxxxxxxxx> wrote:
>
>> Signed-off-by: Pavel Shilovsky <piastry@xxxxxxxxxxx>
>> ---
>> fs/cifs/cifsglob.h | 6 ++++++
>> fs/cifs/connect.c | 27 ++++-----------------------
>> fs/cifs/smb1ops.c | 26 ++++++++++++++++++++++++++
>> 3 files changed, 36 insertions(+), 23 deletions(-)
>>
>> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
>> index 080dd86..7b3e8fe 100644
>> --- a/fs/cifs/cifsglob.h
>> +++ b/fs/cifs/cifsglob.h
>> @@ -170,6 +170,12 @@ struct smb_version_operations {
>> unsigned int (*read_data_offset)(char *);
>> unsigned int (*read_data_length)(char *);
>> int (*map_error)(char *, bool);
>> + struct mid_q_entry * (*find_mid)(struct TCP_Server_Info *, char *);
>> +#ifdef CONFIG_CIFS_DEBUG2
>> + void (*dump_detail)(void *);
>> +#endif
>> + int (*check_message)(char *, unsigned int);
>> + bool (*is_oplock_break)(char *, struct TCP_Server_Info *);
>> };
>>
>> struct smb_version_values {
>> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
>> index 65ec6ef..ce033d7 100644
>> --- a/fs/cifs/connect.c
>> +++ b/fs/cifs/connect.c
>> @@ -783,25 +783,6 @@ is_smb_response(struct TCP_Server_Info *server, unsigned char type)
>> return false;
>> }
>>
>> -static struct mid_q_entry *
>> -find_mid(struct TCP_Server_Info *server, char *buffer)
>> -{
>> - struct smb_hdr *buf = (struct smb_hdr *)buffer;
>> - struct mid_q_entry *mid;
>> -
>> - spin_lock(&GlobalMid_Lock);
>> - list_for_each_entry(mid, &server->pending_mid_q, qhead) {
>> - if (mid->mid == buf->Mid &&
>> - mid->mid_state == MID_REQUEST_SUBMITTED &&
>> - le16_to_cpu(mid->command) == buf->Command) {
>> - spin_unlock(&GlobalMid_Lock);
>> - return mid;
>> - }
>> - }
>> - spin_unlock(&GlobalMid_Lock);
>> - return NULL;
>> -}
>> -
>> void
>> dequeue_mid(struct mid_q_entry *mid, bool malformed)
>> {
>> @@ -986,7 +967,7 @@ standard_receive3(struct TCP_Server_Info *server, struct mid_q_entry *mid)
>> * 48 bytes is enough to display the header and a little bit
>> * into the payload for debugging purposes.
>> */
>> - length = checkSMB(buf, server->total_read);
>> + length = server->ops->check_message(buf, server->total_read);
>> if (length != 0)
>> cifs_dump_mem("Bad SMB: ", buf,
>> min_t(unsigned int, server->total_read, 48));
>> @@ -1059,7 +1040,7 @@ cifs_demultiplex_thread(void *p)
>> continue;
>> server->total_read += length;
>>
>> - mid_entry = find_mid(server, buf);
>> + mid_entry = server->ops->find_mid(server, buf);
>>
>> if (!mid_entry || !mid_entry->receive)
>> length = standard_receive3(server, mid_entry);
>> @@ -1076,13 +1057,13 @@ cifs_demultiplex_thread(void *p)
>> if (mid_entry != NULL) {
>> if (!mid_entry->multiRsp || mid_entry->multiEnd)
>> mid_entry->callback(mid_entry);
>> - } else if (!is_valid_oplock_break(buf, server)) {
>> + } else if (!server->ops->is_oplock_break(buf, server)) {
>> cERROR(1, "No task to wake, unknown frame received! "
>> "NumMids %d", atomic_read(&midCount));
>> cifs_dump_mem("Received Data is: ", buf,
>> HEADER_SIZE(server));
>> #ifdef CONFIG_CIFS_DEBUG2
>> - cifs_dump_detail(buf);
>> + server->ops->dump_detail(buf);
>> cifs_dump_mids(server);
>> #endif /* CIFS_DEBUG2 */
>>
>
>
> Hmmm, might it be cleaner to unconditionally add the dump_detail
> operation to smb_version_operations, and only set it when
> CONFIG_CIFS_DEBUG2 is enabled?
>
> With that, you could get rid of the #ifdef clutter in the code above...
Ok. In this case we should slightly fix cifs_dump_mids that is between
#ifdef statement too.
>
>
>> diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c
>> index 31a6111..8a0b35b 100644
>> --- a/fs/cifs/smb1ops.c
>> +++ b/fs/cifs/smb1ops.c
>> @@ -22,6 +22,7 @@
>> #include "cifs_debug.h"
>> #include "cifspdu.h"
>> #include "cifsproto.h"
>> +#include "cifs_debug.h"
>>
>> /*
>> * An NT cancel request header looks just like the original request except:
>> @@ -82,6 +83,25 @@ cifs_read_data_length(char *buf)
>> le16_to_cpu(rsp->DataLength);
>> }
>>
>> +static struct mid_q_entry *
>> +cifs_find_mid(struct TCP_Server_Info *server, char *buffer)
>> +{
>> + struct smb_hdr *buf = (struct smb_hdr *)buffer;
>> + struct mid_q_entry *mid;
>> +
>> + spin_lock(&GlobalMid_Lock);
>> + list_for_each_entry(mid, &server->pending_mid_q, qhead) {
>> + if (mid->mid == buf->Mid &&
>> + mid->mid_state == MID_REQUEST_SUBMITTED &&
>> + le16_to_cpu(mid->command) == buf->Command) {
>> + spin_unlock(&GlobalMid_Lock);
>> + return mid;
>> + }
>> + }
>> + spin_unlock(&GlobalMid_Lock);
>> + return NULL;
>> +}
>> +
>> struct smb_version_operations smb1_operations = {
>> .send_cancel = send_nt_cancel,
>> .compare_fids = cifs_compare_fids,
>> @@ -90,6 +110,12 @@ struct smb_version_operations smb1_operations = {
>> .read_data_offset = cifs_read_data_offset,
>> .read_data_length = cifs_read_data_length,
>> .map_error = map_smb_to_linux_error,
>> + .find_mid = cifs_find_mid,
>> + .check_message = checkSMB,
>> +#ifdef CONFIG_CIFS_DEBUG2
>> + .dump_detail = cifs_dump_detail,
>> +#endif
>> + .is_oplock_break = is_valid_oplock_break,
>> };
>>
>> struct smb_version_values smb1_values = {
>
>
> --
> Jeff Layton <jlayton@xxxxxxxxxx>
--
Best regards,
Pavel Shilovsky.
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
