Skip to contentSkip to conversation

fnic: remove casting vmalloc

PATCH In review Vasyl Gomonovych 2 participants 2 messages Jul 18, 2019

Subsystems (2)
SCSI Cisco FCoE HBA driver
lore
Share & tools
lore

Vasyl Gomonovych

author
#0 lore raw

PATCH 1/1 fnic: remove casting vmalloc

Generated by: alloc_cast.cocci

Signed-off-by Vasyl Gomonovych <[email protected]>
---
 drivers/scsi/fnic/fnic_debugfs.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/scsi/fnic/fnic_debugfs.c +1 -2
index 21991c99db7c..8c5bae6165d6 100644
@@ -59,8 +59,7 @@ int fnic_debugfs_init(void)
 						fnic_trace_debugfs_root);
 
 	/* Allocate memory to structure */
-	fc_trc_flag = (struct fc_trace_flag_type *)
-		vmalloc(sizeof(struct fc_trace_flag_type));
+	fc_trc_flag = vmalloc(sizeof(struct fc_trace_flag_type));
 
 	if (fc_trc_flag) {
 		fc_trc_flag->fc_row_file = 0;
Signature · 1 line
2.17.1
#1 lore raw
Quoted 1 line · Vasyl Gomonovych

Generated by: alloc_cast.cocci

Please look deeper than just the cocci output.

drivers/scsi/fnic/fnic_debugfs.c

Quoted 1 line
drivers/scsi/fnic/fnic_debugfs.c +0 -0

[]

drivers/scsi/fnic/fnic_debugfs.c int fnic_debugfs_init(void) · line 65

Quoted 10 lines
drivers/scsi/fnic/fnic_debugfs.c +1 -2
@@ -59,8 +59,7 @@ int fnic_debugfs_init(void)
 						fnic_trace_debugfs_root);
 
 	/* Allocate memory to structure */
-	fc_trc_flag = (struct fc_trace_flag_type *)
-		vmalloc(sizeof(struct fc_trace_flag_type));
+	fc_trc_flag = vmalloc(sizeof(struct fc_trace_flag_type));
 
 	if (fc_trc_flag) {
 		fc_trc_flag->fc_row_file = 0;

Not your fault, but this code is not well written.

struct fc_trace_flag_type is:

drivers/scsi/fnic/fnic_debugfs.c:struct fc_trace_flag_type {

drivers/scsi/fnic/fnic_debugfs.c-       u8 fc_row_file;
drivers/scsi/fnic/fnic_debugfs.c-       u8 fc_normal_file;
drivers/scsi/fnic/fnic_debugfs.c-       u8 fnic_trace;
drivers/scsi/fnic/fnic_debugfs.c-       u8 fc_trace;
drivers/scsi/fnic/fnic_debugfs.c-       u8 fc_clear;

drivers/scsi/fnic/fnic_debugfs.c-};

It doesn't require vmalloc as it's just 5 bytes in size. Might as well do either of allocate it with kmalloc or because it's a singleton, just create a static.

Maybe:

---
 drivers/scsi/fnic/fnic_debugfs.c | 55 ++++++++++++++++------------------------
 1 file changed, 22 insertions(+), 33 deletions(-)
drivers/scsi/fnic/fnic_debugfs.c +22 -33
index 21991c99db7c..72071bd4c9a1 100644
@@ -31,15 +31,13 @@ static struct dentry *fnic_fc_rdata_trace_debugfs_file;
 static struct dentry *fnic_fc_trace_enable;
 static struct dentry *fnic_fc_trace_clear;
 
-struct fc_trace_flag_type {
+static struct {
 	u8 fc_row_file;
 	u8 fc_normal_file;
 	u8 fnic_trace;
 	u8 fc_trace;
 	u8 fc_clear;
-};
-
-static struct fc_trace_flag_type *fc_trc_flag;
+} fc_trc_flag;
 
 /*
  * fnic_debugfs_init - Initialize debugfs for fnic debug logging

@@ -52,26 +50,18 @@ static struct fc_trace_flag_type *fc_trc_flag;
  */
 int fnic_debugfs_init(void)
 {
-	int rc = -1;
 	fnic_trace_debugfs_root = debugfs_create_dir("fnic", NULL);
 
 	fnic_stats_debugfs_root = debugfs_create_dir("statistics",
 						fnic_trace_debugfs_root);
 
-	/* Allocate memory to structure */
-	fc_trc_flag = (struct fc_trace_flag_type *)
-		vmalloc(sizeof(struct fc_trace_flag_type));
-
-	if (fc_trc_flag) {
-		fc_trc_flag->fc_row_file = 0;
-		fc_trc_flag->fc_normal_file = 1;
-		fc_trc_flag->fnic_trace = 2;
-		fc_trc_flag->fc_trace = 3;
-		fc_trc_flag->fc_clear = 4;
-	}
+	fc_trc_flag.fc_row_file = 0;
+	fc_trc_flag.fc_normal_file = 1;
+	fc_trc_flag.fnic_trace = 2;
+	fc_trc_flag.fc_trace = 3;
+	fc_trc_flag.fc_clear = 4;
 
-	rc = 0;
-	return rc;
+	return 0;
 }
 
 /*

@@ -89,8 +79,7 @@ void fnic_debugfs_terminate(void)
 	debugfs_remove(fnic_trace_debugfs_root);
 	fnic_trace_debugfs_root = NULL;
 
-	if (fc_trc_flag)
-		vfree(fc_trc_flag);
+	memset(&fc_trc_flag, 0, sizeof(fc_trc_flag));
 }
 
 /*

@@ -121,11 +110,11 @@ static ssize_t fnic_trace_ctrl_read(struct file *filp,
 	u8 *trace_type;
 	len = 0;
 	trace_type = (u8 *)filp->private_data;
-	if (*trace_type == fc_trc_flag->fnic_trace)
+	if (*trace_type == fc_trc_flag.fnic_trace)
 		len = sprintf(buf, "%u\n", fnic_tracing_enabled);
-	else if (*trace_type == fc_trc_flag->fc_trace)
+	else if (*trace_type == fc_trc_flag.fc_trace)
 		len = sprintf(buf, "%u\n", fnic_fc_tracing_enabled);
-	else if (*trace_type == fc_trc_flag->fc_clear)
+	else if (*trace_type == fc_trc_flag.fc_clear)
 		len = sprintf(buf, "%u\n", fnic_fc_trace_cleared);
 	else
 		pr_err("fnic: Cannot read to any debugfs file\n");

@@ -172,11 +161,11 @@ static ssize_t fnic_trace_ctrl_write(struct file *filp,
 	if (ret < 0)
 		return ret;
 
-	if (*trace_type == fc_trc_flag->fnic_trace)
+	if (*trace_type == fc_trc_flag.fnic_trace)
 		fnic_tracing_enabled = val;
-	else if (*trace_type == fc_trc_flag->fc_trace)
+	else if (*trace_type == fc_trc_flag.fc_trace)
 		fnic_fc_tracing_enabled = val;
-	else if (*trace_type == fc_trc_flag->fc_clear)
+	else if (*trace_type == fc_trc_flag.fc_clear)
 		fnic_fc_trace_cleared = val;
 	else
 		pr_err("fnic: cannot write to any debugfs file\n");

@@ -218,7 +207,7 @@ static int fnic_trace_debugfs_open(struct inode *inode,
 	if (!fnic_dbg_prt)
 		return -ENOMEM;
 
-	if (*rdata_ptr == fc_trc_flag->fnic_trace) {
+	if (*rdata_ptr == fc_trc_flag.fnic_trace) {
 		fnic_dbg_prt->buffer = vmalloc(array3_size(3, trace_max_pages,
 							   PAGE_SIZE));
 		if (!fnic_dbg_prt->buffer) {

@@ -347,13 +336,13 @@ void fnic_trace_debugfs_init(void)
 	fnic_trace_enable = debugfs_create_file("tracing_enable",
 					S_IFREG|S_IRUGO|S_IWUSR,
 					fnic_trace_debugfs_root,
-					&(fc_trc_flag->fnic_trace),
+					&fc_trc_flag.fnic_trace,
 					&fnic_trace_ctrl_fops);
 
 	fnic_trace_debugfs_file = debugfs_create_file("trace",
 					S_IFREG|S_IRUGO|S_IWUSR,
 					fnic_trace_debugfs_root,
-					&(fc_trc_flag->fnic_trace),
+					&fc_trc_flag.fnic_trace,
 					&fnic_trace_debugfs_fops);
 }
 

@@ -390,27 +379,27 @@ void fnic_fc_trace_debugfs_init(void)
 	fnic_fc_trace_enable = debugfs_create_file("fc_trace_enable",
 					S_IFREG|S_IRUGO|S_IWUSR,
 					fnic_trace_debugfs_root,
-					&(fc_trc_flag->fc_trace),
+					&fc_trc_flag.fc_trace,
 					&fnic_trace_ctrl_fops);
 
 	fnic_fc_trace_clear = debugfs_create_file("fc_trace_clear",
 					S_IFREG|S_IRUGO|S_IWUSR,
 					fnic_trace_debugfs_root,
-					&(fc_trc_flag->fc_clear),
+					&fc_trc_flag.fc_clear,
 					&fnic_trace_ctrl_fops);
 
 	fnic_fc_rdata_trace_debugfs_file =
 		debugfs_create_file("fc_trace_rdata",
 				    S_IFREG|S_IRUGO|S_IWUSR,
 				    fnic_trace_debugfs_root,
-				    &(fc_trc_flag->fc_normal_file),
+				    &fc_trc_flag.fc_normal_file,
 				    &fnic_trace_debugfs_fops);
 
 	fnic_fc_trace_debugfs_file =
 		debugfs_create_file("fc_trace",
 				    S_IFREG|S_IRUGO|S_IWUSR,
 				    fnic_trace_debugfs_root,
-				    &(fc_trc_flag->fc_row_file),
+				    &fc_trc_flag.fc_row_file,
 				    &fnic_trace_debugfs_fops);
 }