* Re: [PATCH] fnic: remove casting vmalloc
2019-07-18 19:02 [PATCH] fnic: remove casting vmalloc Vasyl Gomonovych
@ 2019-07-18 19:36 ` Joe Perches
0 siblings, 0 replies; 2+ messages in thread
From: Joe Perches @ 2019-07-18 19:36 UTC (permalink / raw)
To: Vasyl Gomonovych, satishkh, sebaddel, kartilak, jejb,
martin.petersen, linux-scsi
Cc: linux-kernel
On Thu, 2019-07-18 at 21:02 +0200, Vasyl Gomonovych wrote:
> Generated by: alloc_cast.cocci
Please look deeper than just the cocci output.
> diff --git a/drivers/scsi/fnic/fnic_debugfs.c b/drivers/scsi/fnic/fnic_debugfs.c
[]
> @@ -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(-)
diff --git a/drivers/scsi/fnic/fnic_debugfs.c b/drivers/scsi/fnic/fnic_debugfs.c
index 21991c99db7c..72071bd4c9a1 100644
--- a/drivers/scsi/fnic/fnic_debugfs.c
+++ b/drivers/scsi/fnic/fnic_debugfs.c
@@ -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);
}
^ permalink raw reply related [flat|nested] 2+ messages in thread