All of lore.kernel.org
 help / color / mirror / Atom feed
From: walter harms <wharms@bfs.de>
To: Dan Carpenter <error27@gmail.com>,
	"Nicholas A. Bellinger" <nab@linux-iscsi.org>,
	James Bottomley <James.Bottomley@suse.de>,
	linux-kernel@vger.kernel.org, linux-scsi@vger.kernel.org,
	kernel-janitors@vger.kernel.org
Subject: Re: [patch] [SCSI] target: dubious one-bit signed bitfield
Date: Sat, 15 Jan 2011 16:36:11 +0000	[thread overview]
Message-ID: <4D31CCFB.1060504@bfs.de> (raw)
In-Reply-To: <20110115140445.GD2721@bicker>



Am 15.01.2011 15:04, schrieb Dan Carpenter:
> The signed one-bit types can be 0 or -1 which can cause a problem if
> someone ever checks if (foo->lu_gp_assoc = 1).  The current code is
> fine because everyone just checks zero vs non-zero.  But Sparse
> complains about it so lets change it.  The warnings look like this:
> 

Your code looks ok,
but to avoid that kind of errors it may be better to use int here.

re,
 wh



> include/target/target_core_base.h:228:26: error: dubious one-bit signed bitfield
> 
> Signed-off-by: Dan Carpenter <error27@gmail.com>
> 
> diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h
> index 07fdfb6..764177b 100644
> --- a/include/target/target_core_base.h
> +++ b/include/target/target_core_base.h
> @@ -225,7 +225,7 @@ struct t10_alua_lu_gp {
>  } ____cacheline_aligned;
>  
>  struct t10_alua_lu_gp_member {
> -	int lu_gp_assoc:1;
> +	bool lu_gp_assoc;
>  	atomic_t lu_gp_mem_ref_cnt;
>  	spinlock_t lu_gp_mem_lock;
>  	struct t10_alua_lu_gp *lu_gp;
> @@ -257,7 +257,7 @@ struct t10_alua_tg_pt_gp {
>  } ____cacheline_aligned;
>  
>  struct t10_alua_tg_pt_gp_member {
> -	int tg_pt_gp_assoc:1;
> +	bool tg_pt_gp_assoc;
>  	atomic_t tg_pt_gp_mem_ref_cnt;
>  	spinlock_t tg_pt_gp_mem_lock;
>  	struct t10_alua_tg_pt_gp *tg_pt_gp;
> @@ -322,7 +322,7 @@ struct t10_pr_registration {
>  	int pr_res_type;
>  	int pr_res_scope;
>  	/* Used for fabric initiator WWPNs using a ISID */
> -	int isid_present_at_reg:1;
> +	bool isid_present_at_reg;
>  	u32 pr_res_mapped_lun;
>  	u32 pr_aptpl_target_lun;
>  	u32 pr_res_generation;
> @@ -404,7 +404,7 @@ struct se_transport_task {
>  	unsigned long long	t_task_lba;
>  	int			t_tasks_failed;
>  	int			t_tasks_fua;
> -	int			t_tasks_bidi:1;
> +	bool			t_tasks_bidi;
>  	u32			t_task_cdbs;
>  	u32			t_tasks_check;
>  	u32			t_tasks_no;
> @@ -456,7 +456,7 @@ struct se_task {
>  	u8		task_flags;
>  	int		task_error_status;
>  	int		task_state_flags;
> -	int		task_padded_sg:1;
> +	bool		task_padded_sg;
>  	unsigned long long	task_lba;
>  	u32		task_no;
>  	u32		task_sectors;
> @@ -569,7 +569,7 @@ struct se_ua {
>  struct se_node_acl {
>  	char			initiatorname[TRANSPORT_IQN_LEN];
>  	/* Used to signal demo mode created ACL, disabled by default */
> -	int			dynamic_node_acl:1;
> +	bool			dynamic_node_acl;
>  	u32			queue_depth;
>  	u32			acl_index;
>  	u64			num_cmds;
> @@ -622,7 +622,7 @@ struct se_lun_acl {
>  }  ____cacheline_aligned;
>  
>  struct se_dev_entry {
> -	int			def_pr_registered:1;
> +	bool			def_pr_registered;
>  	/* See transport_lunflags_table */
>  	u32			lun_flags;
>  	u32			deve_cmds;
> diff --git a/include/target/target_core_fabric_ops.h b/include/target/target_core_fabric_ops.h
> index f3ac12b..5eb8b1a 100644
> --- a/include/target/target_core_fabric_ops.h
> +++ b/include/target/target_core_fabric_ops.h
> @@ -8,7 +8,7 @@ struct target_core_fabric_ops {
>  	 * for scatterlist chaining using transport_do_task_sg_link(),
>  	 * disabled by default
>  	 */
> -	int task_sg_chaining:1;
> +	bool task_sg_chaining;
>  	char *(*get_fabric_name)(void);
>  	u8 (*get_fabric_proto_ident)(struct se_portal_group *);
>  	char *(*tpg_get_wwn)(struct se_portal_group *);
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 

WARNING: multiple messages have this Message-ID (diff)
From: walter harms <wharms@bfs.de>
To: Dan Carpenter <error27@gmail.com>,
	"Nicholas A. Bellinger" <nab@linux-iscsi.org>,
	James Bottomley <James.Bottomley@suse.de>,
	linux-kernel@vger.kernel.org, linux-scsi@vger.kernel.org,
	kernel-janitors@vger.kernel.org
Subject: Re: [patch] [SCSI] target: dubious one-bit signed bitfield
Date: Sat, 15 Jan 2011 17:36:11 +0100	[thread overview]
Message-ID: <4D31CCFB.1060504@bfs.de> (raw)
In-Reply-To: <20110115140445.GD2721@bicker>



Am 15.01.2011 15:04, schrieb Dan Carpenter:
> The signed one-bit types can be 0 or -1 which can cause a problem if
> someone ever checks if (foo->lu_gp_assoc == 1).  The current code is
> fine because everyone just checks zero vs non-zero.  But Sparse
> complains about it so lets change it.  The warnings look like this:
> 

Your code looks ok,
but to avoid that kind of errors it may be better to use int here.

re,
 wh



> include/target/target_core_base.h:228:26: error: dubious one-bit signed bitfield
> 
> Signed-off-by: Dan Carpenter <error27@gmail.com>
> 
> diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h
> index 07fdfb6..764177b 100644
> --- a/include/target/target_core_base.h
> +++ b/include/target/target_core_base.h
> @@ -225,7 +225,7 @@ struct t10_alua_lu_gp {
>  } ____cacheline_aligned;
>  
>  struct t10_alua_lu_gp_member {
> -	int lu_gp_assoc:1;
> +	bool lu_gp_assoc;
>  	atomic_t lu_gp_mem_ref_cnt;
>  	spinlock_t lu_gp_mem_lock;
>  	struct t10_alua_lu_gp *lu_gp;
> @@ -257,7 +257,7 @@ struct t10_alua_tg_pt_gp {
>  } ____cacheline_aligned;
>  
>  struct t10_alua_tg_pt_gp_member {
> -	int tg_pt_gp_assoc:1;
> +	bool tg_pt_gp_assoc;
>  	atomic_t tg_pt_gp_mem_ref_cnt;
>  	spinlock_t tg_pt_gp_mem_lock;
>  	struct t10_alua_tg_pt_gp *tg_pt_gp;
> @@ -322,7 +322,7 @@ struct t10_pr_registration {
>  	int pr_res_type;
>  	int pr_res_scope;
>  	/* Used for fabric initiator WWPNs using a ISID */
> -	int isid_present_at_reg:1;
> +	bool isid_present_at_reg;
>  	u32 pr_res_mapped_lun;
>  	u32 pr_aptpl_target_lun;
>  	u32 pr_res_generation;
> @@ -404,7 +404,7 @@ struct se_transport_task {
>  	unsigned long long	t_task_lba;
>  	int			t_tasks_failed;
>  	int			t_tasks_fua;
> -	int			t_tasks_bidi:1;
> +	bool			t_tasks_bidi;
>  	u32			t_task_cdbs;
>  	u32			t_tasks_check;
>  	u32			t_tasks_no;
> @@ -456,7 +456,7 @@ struct se_task {
>  	u8		task_flags;
>  	int		task_error_status;
>  	int		task_state_flags;
> -	int		task_padded_sg:1;
> +	bool		task_padded_sg;
>  	unsigned long long	task_lba;
>  	u32		task_no;
>  	u32		task_sectors;
> @@ -569,7 +569,7 @@ struct se_ua {
>  struct se_node_acl {
>  	char			initiatorname[TRANSPORT_IQN_LEN];
>  	/* Used to signal demo mode created ACL, disabled by default */
> -	int			dynamic_node_acl:1;
> +	bool			dynamic_node_acl;
>  	u32			queue_depth;
>  	u32			acl_index;
>  	u64			num_cmds;
> @@ -622,7 +622,7 @@ struct se_lun_acl {
>  }  ____cacheline_aligned;
>  
>  struct se_dev_entry {
> -	int			def_pr_registered:1;
> +	bool			def_pr_registered;
>  	/* See transport_lunflags_table */
>  	u32			lun_flags;
>  	u32			deve_cmds;
> diff --git a/include/target/target_core_fabric_ops.h b/include/target/target_core_fabric_ops.h
> index f3ac12b..5eb8b1a 100644
> --- a/include/target/target_core_fabric_ops.h
> +++ b/include/target/target_core_fabric_ops.h
> @@ -8,7 +8,7 @@ struct target_core_fabric_ops {
>  	 * for scatterlist chaining using transport_do_task_sg_link(),
>  	 * disabled by default
>  	 */
> -	int task_sg_chaining:1;
> +	bool task_sg_chaining;
>  	char *(*get_fabric_name)(void);
>  	u8 (*get_fabric_proto_ident)(struct se_portal_group *);
>  	char *(*tpg_get_wwn)(struct se_portal_group *);
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 

  reply	other threads:[~2011-01-15 16:36 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-01-15 14:04 [patch] [SCSI] target: dubious one-bit signed bitfield Dan Carpenter
2011-01-15 14:04 ` Dan Carpenter
2011-01-15 16:36 ` walter harms [this message]
2011-01-15 16:36   ` walter harms
2011-01-15 16:54   ` Dan Carpenter
2011-01-15 16:54     ` Dan Carpenter
2011-01-17  8:08     ` walter harms
2011-01-17  8:08       ` walter harms
2011-01-15 20:53 ` Nicholas A. Bellinger
2011-01-15 20:53   ` Nicholas A. Bellinger

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4D31CCFB.1060504@bfs.de \
    --to=wharms@bfs.de \
    --cc=James.Bottomley@suse.de \
    --cc=error27@gmail.com \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=nab@linux-iscsi.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.