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"
???
