lkml.org 
[lkml]   [2021]   [Jun]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v4 02/10] cifsd: add server handler
On Wed, Jun 02, 2021 at 12:48:39PM +0900, Namjae Jeon wrote:
> +/* @FIXME clean up this code */

Hmm, should that be in a submitted codebase?

> +#define DATA_STREAM 1
> +#define DIR_STREAM 2

Should this use a named enum to document the usage a bit better?

> +#ifndef ksmbd_pr_fmt
> +#ifdef SUBMOD_NAME
> +#define ksmbd_pr_fmt(fmt) "ksmbd: " SUBMOD_NAME ": " fmt
> +#else
> +#define ksmbd_pr_fmt(fmt) "ksmbd: " fmt
> +#endif
> +#endif

Why not use the pr_fmt built into pr_*? With that all the message
wrappers except for the _dbg one can go away.

Also can you please decided if this is kcifsd or ksmbd? Using both
names is rather confusing.

> +#ifndef ____ksmbd_align
> +#define ____ksmbd_align __aligned(4)
> +#endif

No need for the ifndef and all the _ prefixes. More importantly from
a quick look it seems like none of the structures really needs the
attribute anyway.

> +#define KSMBD_SHARE_CONFIG_PATH(s) \
> + ({ \
> + char *p = (s)->____payload; \
> + if ((s)->veto_list_sz) \
> + p += (s)->veto_list_sz + 1; \
> + p; \
> + })

Why no inline function?

> +/* @FIXME */
> +#include "ksmbd_server.h"

???

\
 
 \ /
  Last update: 2021-06-15 10:11    [W:0.859 / U:0.220 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site