Re: [PATCH] cifs: Fix bool initialization/comparison

[Nico Kadel-Garcia]

On Sat, Oct 7, 2017 at 10:02 AM, Thomas Meyer via samba-technical
 wrote:
> Bool initializations should use true and false. Bool tests don't need
> comparisons.

Except that "==" is not a pure boolean check. It's a value check, and
unless these values are defined *very* carefully they may be set to
non-boolean values.

You may be write on these specific checks that they are, in fact,
booleans. But I'd be be cautious about stripping out such checks as a
matter of style.


> Signed-off-by: Thomas Meyer 
> ---
>
> diff -u -p a/fs/cifs/cifsacl.c b/fs/cifs/cifsacl.c
> --- a/fs/cifs/cifsacl.c
> +++ b/fs/cifs/cifsacl.c
> @@ -370,7 +370,7 @@ sid_to_id(struct cifs_sb_info *cifs_sb,
> else
> is_group = false;
>
> -   if (is_well_known_sid(psid, _id, is_group) == false)
> +   if (!is_well_known_sid(psid, _id, is_group))
> goto try_upcall_to_get_id;
>
> if (is_group) {
> diff -u -p a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
> --- a/fs/cifs/cifssmb.c
> +++ b/fs/cifs/cifssmb.c
> @@ -4508,7 +4508,7 @@ findFirstRetry:
> psrch_inf->unicode = false;
>
> psrch_inf->ntwrk_buf_start = (char *)pSMBr;
> -   psrch_inf->smallBuf = 0;
> +   psrch_inf->smallBuf = false;
> psrch_inf->srch_entries_start =
> (char *) >hdr.Protocol +
> le16_to_cpu(pSMBr->t2.DataOffset);
> @@ -4642,7 +4642,7 @@ int CIFSFindNext(const unsigned int xid,
> cifs_buf_release(psrch_inf->ntwrk_buf_start);
> psrch_inf->srch_entries_start = response_data;
> psrch_inf->ntwrk_buf_start = (char *)pSMB;
> -   psrch_inf->smallBuf = 0;
> +   psrch_inf->smallBuf = false;
> if (parms->EndofSearch)
> psrch_inf->endOfSearch = true;
> else
> diff -u -p a/fs/cifs/connect.c b/fs/cifs/connect.c
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -1081,7 +1081,7 @@ static int cifs_parse_security_flavors(c
> break;
>  #endif
> case Opt_sec_none:
> -   vol->nullauth = 1;
> +   vol->nullauth = true;
> break;
> default:
> cifs_dbg(VFS, "bad security option: %s\n", value);
> @@ -1265,9 +1265,9 @@ cifs_parse_mount_options(const char *mou
>
> /* vol->retry default is 0 (i.e. "soft" limited retry not hard retry) 
> */
> /* default is always to request posix paths. */
> -   vol->posix_paths = 1;
> +   vol->posix_paths = true;
> /* default to using server inode numbers where available */
> -   vol->server_ino = 1;
> +   vol->server_ino = true;
>
> /* default is to use strict cifs caching semantics */
> vol->strict_io = true;
> @@ -1333,10 +1333,10 @@ cifs_parse_mount_options(const char *mou
>
> /* Boolean values */
> case Opt_user_xattr:
> -   vol->no_xattr = 0;
> +   vol->no_xattr = false;
> break;
> case Opt_nouser_xattr:
> -   vol->no_xattr = 1;
> +   vol->no_xattr = true;
> break;
> case Opt_forceuid:
> override_uid = 1;
> @@ -1351,22 +1351,22 @@ cifs_parse_mount_options(const char *mou
> override_gid = 0;
> break;
> case Opt_noblocksend:
> -   vol->noblocksnd = 1;
> +   vol->noblocksnd = true;
> break;
> case Opt_noautotune:
> -   vol->noautotune = 1;
> +   vol->noautotune = true;
> break;
> case Opt_hard:
> -   vol->retry = 1;
> +   vol->retry = true;
> break;
> case Opt_soft:
> -   vol->retry = 0;
> +   vol->retry = false;
> break;
> case Opt_perm:
> -   vol->noperm = 0;
> +   vol->noperm = false;
> break;
> case Opt_noperm:
> -   vol->noperm = 1;
> +   vol->noperm = true;
> break;
> case Opt_mapchars:
> vol->sfu_remap = true;
> @@ -1383,31 +1383,31 @@ cifs_parse_mount_options(const char *mou
> vol->remap = false;
> break;
> case Opt_sfu:
> -   

Re: [PATCH] cifs: Fix bool initialization/comparison

[Nico Kadel-Garcia]

On Sat, Oct 7, 2017 at 10:02 AM, Thomas Meyer via samba-technical
 wrote:
> Bool initializations should use true and false. Bool tests don't need
> comparisons.

Except that "==" is not a pure boolean check. It's a value check, and
unless these values are defined *very* carefully they may be set to
non-boolean values.

You may be write on these specific checks that they are, in fact,
booleans. But I'd be be cautious about stripping out such checks as a
matter of style.


> Signed-off-by: Thomas Meyer 
> ---
>
> diff -u -p a/fs/cifs/cifsacl.c b/fs/cifs/cifsacl.c
> --- a/fs/cifs/cifsacl.c
> +++ b/fs/cifs/cifsacl.c
> @@ -370,7 +370,7 @@ sid_to_id(struct cifs_sb_info *cifs_sb,
> else
> is_group = false;
>
> -   if (is_well_known_sid(psid, _id, is_group) == false)
> +   if (!is_well_known_sid(psid, _id, is_group))
> goto try_upcall_to_get_id;
>
> if (is_group) {
> diff -u -p a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
> --- a/fs/cifs/cifssmb.c
> +++ b/fs/cifs/cifssmb.c
> @@ -4508,7 +4508,7 @@ findFirstRetry:
> psrch_inf->unicode = false;
>
> psrch_inf->ntwrk_buf_start = (char *)pSMBr;
> -   psrch_inf->smallBuf = 0;
> +   psrch_inf->smallBuf = false;
> psrch_inf->srch_entries_start =
> (char *) >hdr.Protocol +
> le16_to_cpu(pSMBr->t2.DataOffset);
> @@ -4642,7 +4642,7 @@ int CIFSFindNext(const unsigned int xid,
> cifs_buf_release(psrch_inf->ntwrk_buf_start);
> psrch_inf->srch_entries_start = response_data;
> psrch_inf->ntwrk_buf_start = (char *)pSMB;
> -   psrch_inf->smallBuf = 0;
> +   psrch_inf->smallBuf = false;
> if (parms->EndofSearch)
> psrch_inf->endOfSearch = true;
> else
> diff -u -p a/fs/cifs/connect.c b/fs/cifs/connect.c
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -1081,7 +1081,7 @@ static int cifs_parse_security_flavors(c
> break;
>  #endif
> case Opt_sec_none:
> -   vol->nullauth = 1;
> +   vol->nullauth = true;
> break;
> default:
> cifs_dbg(VFS, "bad security option: %s\n", value);
> @@ -1265,9 +1265,9 @@ cifs_parse_mount_options(const char *mou
>
> /* vol->retry default is 0 (i.e. "soft" limited retry not hard retry) 
> */
> /* default is always to request posix paths. */
> -   vol->posix_paths = 1;
> +   vol->posix_paths = true;
> /* default to using server inode numbers where available */
> -   vol->server_ino = 1;
> +   vol->server_ino = true;
>
> /* default is to use strict cifs caching semantics */
> vol->strict_io = true;
> @@ -1333,10 +1333,10 @@ cifs_parse_mount_options(const char *mou
>
> /* Boolean values */
> case Opt_user_xattr:
> -   vol->no_xattr = 0;
> +   vol->no_xattr = false;
> break;
> case Opt_nouser_xattr:
> -   vol->no_xattr = 1;
> +   vol->no_xattr = true;
> break;
> case Opt_forceuid:
> override_uid = 1;
> @@ -1351,22 +1351,22 @@ cifs_parse_mount_options(const char *mou
> override_gid = 0;
> break;
> case Opt_noblocksend:
> -   vol->noblocksnd = 1;
> +   vol->noblocksnd = true;
> break;
> case Opt_noautotune:
> -   vol->noautotune = 1;
> +   vol->noautotune = true;
> break;
> case Opt_hard:
> -   vol->retry = 1;
> +   vol->retry = true;
> break;
> case Opt_soft:
> -   vol->retry = 0;
> +   vol->retry = false;
> break;
> case Opt_perm:
> -   vol->noperm = 0;
> +   vol->noperm = false;
> break;
> case Opt_noperm:
> -   vol->noperm = 1;
> +   vol->noperm = true;
> break;
> case Opt_mapchars:
> vol->sfu_remap = true;
> @@ -1383,31 +1383,31 @@ cifs_parse_mount_options(const char *mou
> vol->remap = false;
> break;
> case Opt_sfu:
> -   vol->sfu_emul = 1;
> +   

[PATCH] cifs: Fix bool initialization/comparison

[Thomas Meyer]

Bool initializations should use true and false. Bool tests don't need
comparisons.

Signed-off-by: Thomas Meyer 
---

diff -u -p a/fs/cifs/cifsacl.c b/fs/cifs/cifsacl.c
--- a/fs/cifs/cifsacl.c
+++ b/fs/cifs/cifsacl.c
@@ -370,7 +370,7 @@ sid_to_id(struct cifs_sb_info *cifs_sb,
else
is_group = false;
 
-   if (is_well_known_sid(psid, _id, is_group) == false)
+   if (!is_well_known_sid(psid, _id, is_group))
goto try_upcall_to_get_id;
 
if (is_group) {
diff -u -p a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
--- a/fs/cifs/cifssmb.c
+++ b/fs/cifs/cifssmb.c
@@ -4508,7 +4508,7 @@ findFirstRetry:
psrch_inf->unicode = false;
 
psrch_inf->ntwrk_buf_start = (char *)pSMBr;
-   psrch_inf->smallBuf = 0;
+   psrch_inf->smallBuf = false;
psrch_inf->srch_entries_start =
(char *) >hdr.Protocol +
le16_to_cpu(pSMBr->t2.DataOffset);
@@ -4642,7 +4642,7 @@ int CIFSFindNext(const unsigned int xid,
cifs_buf_release(psrch_inf->ntwrk_buf_start);
psrch_inf->srch_entries_start = response_data;
psrch_inf->ntwrk_buf_start = (char *)pSMB;
-   psrch_inf->smallBuf = 0;
+   psrch_inf->smallBuf = false;
if (parms->EndofSearch)
psrch_inf->endOfSearch = true;
else
diff -u -p a/fs/cifs/connect.c b/fs/cifs/connect.c
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -1081,7 +1081,7 @@ static int cifs_parse_security_flavors(c
break;
 #endif
case Opt_sec_none:
-   vol->nullauth = 1;
+   vol->nullauth = true;
break;
default:
cifs_dbg(VFS, "bad security option: %s\n", value);
@@ -1265,9 +1265,9 @@ cifs_parse_mount_options(const char *mou
 
/* vol->retry default is 0 (i.e. "soft" limited retry not hard retry) */
/* default is always to request posix paths. */
-   vol->posix_paths = 1;
+   vol->posix_paths = true;
/* default to using server inode numbers where available */
-   vol->server_ino = 1;
+   vol->server_ino = true;
 
/* default is to use strict cifs caching semantics */
vol->strict_io = true;
@@ -1333,10 +1333,10 @@ cifs_parse_mount_options(const char *mou
 
/* Boolean values */
case Opt_user_xattr:
-   vol->no_xattr = 0;
+   vol->no_xattr = false;
break;
case Opt_nouser_xattr:
-   vol->no_xattr = 1;
+   vol->no_xattr = true;
break;
case Opt_forceuid:
override_uid = 1;
@@ -1351,22 +1351,22 @@ cifs_parse_mount_options(const char *mou
override_gid = 0;
break;
case Opt_noblocksend:
-   vol->noblocksnd = 1;
+   vol->noblocksnd = true;
break;
case Opt_noautotune:
-   vol->noautotune = 1;
+   vol->noautotune = true;
break;
case Opt_hard:
-   vol->retry = 1;
+   vol->retry = true;
break;
case Opt_soft:
-   vol->retry = 0;
+   vol->retry = false;
break;
case Opt_perm:
-   vol->noperm = 0;
+   vol->noperm = false;
break;
case Opt_noperm:
-   vol->noperm = 1;
+   vol->noperm = true;
break;
case Opt_mapchars:
vol->sfu_remap = true;
@@ -1383,31 +1383,31 @@ cifs_parse_mount_options(const char *mou
vol->remap = false;
break;
case Opt_sfu:
-   vol->sfu_emul = 1;
+   vol->sfu_emul = true;
break;
case Opt_nosfu:
-   vol->sfu_emul = 0;
+   vol->sfu_emul = false;
break;
case Opt_nodfs:
-   vol->nodfs = 1;
+   vol->nodfs = true;
break;
case Opt_posixpaths:
-   vol->posix_paths = 1;
+   vol->posix_paths = true;
break;
case Opt_noposixpaths:
-   

[PATCH] cifs: Fix bool initialization/comparison

[Thomas Meyer]

Bool initializations should use true and false. Bool tests don't need
comparisons.

Signed-off-by: Thomas Meyer 
---

diff -u -p a/fs/cifs/cifsacl.c b/fs/cifs/cifsacl.c
--- a/fs/cifs/cifsacl.c
+++ b/fs/cifs/cifsacl.c
@@ -370,7 +370,7 @@ sid_to_id(struct cifs_sb_info *cifs_sb,
else
is_group = false;
 
-   if (is_well_known_sid(psid, _id, is_group) == false)
+   if (!is_well_known_sid(psid, _id, is_group))
goto try_upcall_to_get_id;
 
if (is_group) {
diff -u -p a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
--- a/fs/cifs/cifssmb.c
+++ b/fs/cifs/cifssmb.c
@@ -4508,7 +4508,7 @@ findFirstRetry:
psrch_inf->unicode = false;
 
psrch_inf->ntwrk_buf_start = (char *)pSMBr;
-   psrch_inf->smallBuf = 0;
+   psrch_inf->smallBuf = false;
psrch_inf->srch_entries_start =
(char *) >hdr.Protocol +
le16_to_cpu(pSMBr->t2.DataOffset);
@@ -4642,7 +4642,7 @@ int CIFSFindNext(const unsigned int xid,
cifs_buf_release(psrch_inf->ntwrk_buf_start);
psrch_inf->srch_entries_start = response_data;
psrch_inf->ntwrk_buf_start = (char *)pSMB;
-   psrch_inf->smallBuf = 0;
+   psrch_inf->smallBuf = false;
if (parms->EndofSearch)
psrch_inf->endOfSearch = true;
else
diff -u -p a/fs/cifs/connect.c b/fs/cifs/connect.c
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -1081,7 +1081,7 @@ static int cifs_parse_security_flavors(c
break;
 #endif
case Opt_sec_none:
-   vol->nullauth = 1;
+   vol->nullauth = true;
break;
default:
cifs_dbg(VFS, "bad security option: %s\n", value);
@@ -1265,9 +1265,9 @@ cifs_parse_mount_options(const char *mou
 
/* vol->retry default is 0 (i.e. "soft" limited retry not hard retry) */
/* default is always to request posix paths. */
-   vol->posix_paths = 1;
+   vol->posix_paths = true;
/* default to using server inode numbers where available */
-   vol->server_ino = 1;
+   vol->server_ino = true;
 
/* default is to use strict cifs caching semantics */
vol->strict_io = true;
@@ -1333,10 +1333,10 @@ cifs_parse_mount_options(const char *mou
 
/* Boolean values */
case Opt_user_xattr:
-   vol->no_xattr = 0;
+   vol->no_xattr = false;
break;
case Opt_nouser_xattr:
-   vol->no_xattr = 1;
+   vol->no_xattr = true;
break;
case Opt_forceuid:
override_uid = 1;
@@ -1351,22 +1351,22 @@ cifs_parse_mount_options(const char *mou
override_gid = 0;
break;
case Opt_noblocksend:
-   vol->noblocksnd = 1;
+   vol->noblocksnd = true;
break;
case Opt_noautotune:
-   vol->noautotune = 1;
+   vol->noautotune = true;
break;
case Opt_hard:
-   vol->retry = 1;
+   vol->retry = true;
break;
case Opt_soft:
-   vol->retry = 0;
+   vol->retry = false;
break;
case Opt_perm:
-   vol->noperm = 0;
+   vol->noperm = false;
break;
case Opt_noperm:
-   vol->noperm = 1;
+   vol->noperm = true;
break;
case Opt_mapchars:
vol->sfu_remap = true;
@@ -1383,31 +1383,31 @@ cifs_parse_mount_options(const char *mou
vol->remap = false;
break;
case Opt_sfu:
-   vol->sfu_emul = 1;
+   vol->sfu_emul = true;
break;
case Opt_nosfu:
-   vol->sfu_emul = 0;
+   vol->sfu_emul = false;
break;
case Opt_nodfs:
-   vol->nodfs = 1;
+   vol->nodfs = true;
break;
case Opt_posixpaths:
-   vol->posix_paths = 1;
+   vol->posix_paths = true;
break;
case Opt_noposixpaths:
-   vol->posix_paths = 0;
+