* [PATCH] tracing: use strlcpy instead of strncpy @ 2009-05-18 11:35 Lai Jiangshan 2009-05-18 20:55 ` Jiri Slaby 0 siblings, 1 reply; 6+ messages in thread From: Lai Jiangshan @ 2009-05-18 11:35 UTC (permalink / raw) To: Ingo Molnar; +Cc: Steven Rostedt, Li Zefan, LKML strlcpy() will add '\0' for the copied string. [Impact] cleanup Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com> --- diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c index 05b4747..9359e85 100644 --- a/kernel/trace/blktrace.c +++ b/kernel/trace/blktrace.c @@ -434,8 +434,7 @@ int do_blk_trace_setup(struct request_queue *q, char *name, dev_t dev, if (!buts->buf_size || !buts->buf_nr) return -EINVAL; - strncpy(buts->name, name, BLKTRACE_BDEV_SIZE); - buts->name[BLKTRACE_BDEV_SIZE - 1] = '\0'; + strlcpy(buts->name, name, BLKTRACE_BDEV_SIZE); /* * some device names have larger paths - convert the slashes diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c index 665a915..2484555 100644 --- a/kernel/trace/trace.c +++ b/kernel/trace/trace.c @@ -133,7 +133,7 @@ static char *default_bootup_tracer; static int __init set_ftrace(char *str) { - strncpy(bootup_tracer_buf, str, BOOTUP_TRACER_SIZE); + strlcpy(bootup_tracer_buf, str, BOOTUP_TRACER_SIZE); default_bootup_tracer = bootup_tracer_buf; /* We are using ftrace early, expand it */ ring_buffer_expanded = 1; diff --git a/kernel/trace/trace_branch.c b/kernel/trace/trace_branch.c index 7a7a9fd..a84e036 100644 --- a/kernel/trace/trace_branch.c +++ b/kernel/trace/trace_branch.c @@ -67,10 +67,8 @@ probe_likely_condition(struct ftrace_branch_data *f, int val, int expect) p--; p++; - strncpy(entry->func, f->func, TRACE_FUNC_SIZE); - strncpy(entry->file, p, TRACE_FILE_SIZE); - entry->func[TRACE_FUNC_SIZE] = 0; - entry->file[TRACE_FILE_SIZE] = 0; + strlcpy(entry->func, f->func, TRACE_FUNC_SIZE); + strlcpy(entry->file, p, TRACE_FILE_SIZE); entry->line = f->line; entry->correct = val == expect; ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] tracing: use strlcpy instead of strncpy 2009-05-18 11:35 [PATCH] tracing: use strlcpy instead of strncpy Lai Jiangshan @ 2009-05-18 20:55 ` Jiri Slaby 2009-05-19 1:51 ` Lai Jiangshan 0 siblings, 1 reply; 6+ messages in thread From: Jiri Slaby @ 2009-05-18 20:55 UTC (permalink / raw) To: Lai Jiangshan; +Cc: Ingo Molnar, Steven Rostedt, Li Zefan, LKML On 05/18/2009 01:35 PM, Lai Jiangshan wrote: > strlcpy() will add '\0' for the copied string. > > [Impact] cleanup ... > --- a/kernel/trace/trace_branch.c > +++ b/kernel/trace/trace_branch.c > @@ -67,10 +67,8 @@ probe_likely_condition(struct ftrace_branch_data *f, int val, int expect) > p--; > p++; > > - strncpy(entry->func, f->func, TRACE_FUNC_SIZE); > - strncpy(entry->file, p, TRACE_FILE_SIZE); > - entry->func[TRACE_FUNC_SIZE] = 0; > - entry->file[TRACE_FILE_SIZE] = 0; > + strlcpy(entry->func, f->func, TRACE_FUNC_SIZE); > + strlcpy(entry->file, p, TRACE_FILE_SIZE); Hmm, this is not the same. +1 is missing here. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] tracing: use strlcpy instead of strncpy 2009-05-18 20:55 ` Jiri Slaby @ 2009-05-19 1:51 ` Lai Jiangshan 2009-05-19 12:54 ` Stefan Richter 2009-05-19 23:43 ` Frederic Weisbecker 0 siblings, 2 replies; 6+ messages in thread From: Lai Jiangshan @ 2009-05-19 1:51 UTC (permalink / raw) To: Ingo Molnar; +Cc: Jiri Slaby, Steven Rostedt, Li Zefan, LKML strlcpy() will add '\0' for the copied string. [Impact] cleanup Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com> --- diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c index 05b4747..9359e85 100644 --- a/kernel/trace/blktrace.c +++ b/kernel/trace/blktrace.c @@ -434,8 +434,7 @@ int do_blk_trace_setup(struct request_queue *q, char *name, dev_t dev, if (!buts->buf_size || !buts->buf_nr) return -EINVAL; - strncpy(buts->name, name, BLKTRACE_BDEV_SIZE); - buts->name[BLKTRACE_BDEV_SIZE - 1] = '\0'; + strlcpy(buts->name, name, BLKTRACE_BDEV_SIZE); /* * some device names have larger paths - convert the slashes diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c index 665a915..2484555 100644 --- a/kernel/trace/trace.c +++ b/kernel/trace/trace.c @@ -133,7 +133,7 @@ static char *default_bootup_tracer; static int __init set_ftrace(char *str) { - strncpy(bootup_tracer_buf, str, BOOTUP_TRACER_SIZE); + strlcpy(bootup_tracer_buf, str, BOOTUP_TRACER_SIZE); default_bootup_tracer = bootup_tracer_buf; /* We are using ftrace early, expand it */ ring_buffer_expanded = 1; diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h index 6e735d4..e08c952 100644 --- a/kernel/trace/trace.h +++ b/kernel/trace/trace.h @@ -155,8 +155,8 @@ struct trace_boot_ret { struct trace_branch { struct trace_entry ent; unsigned line; - char func[TRACE_FUNC_SIZE+1]; - char file[TRACE_FILE_SIZE+1]; + char func[TRACE_FUNC_SIZE]; + char file[TRACE_FILE_SIZE]; char correct; }; diff --git a/kernel/trace/trace_branch.c b/kernel/trace/trace_branch.c index 7a7a9fd..a84e036 100644 --- a/kernel/trace/trace_branch.c +++ b/kernel/trace/trace_branch.c @@ -67,10 +67,8 @@ probe_likely_condition(struct ftrace_branch_data *f, int val, int expect) p--; p++; - strncpy(entry->func, f->func, TRACE_FUNC_SIZE); - strncpy(entry->file, p, TRACE_FILE_SIZE); - entry->func[TRACE_FUNC_SIZE] = 0; - entry->file[TRACE_FILE_SIZE] = 0; + strlcpy(entry->func, f->func, TRACE_FUNC_SIZE); + strlcpy(entry->file, p, TRACE_FILE_SIZE); entry->line = f->line; entry->correct = val == expect; diff --git a/kernel/trace/trace_event_types.h b/kernel/trace/trace_event_types.h index 5e32e37..2ae6679 100644 --- a/kernel/trace/trace_event_types.h +++ b/kernel/trace/trace_event_types.h @@ -122,10 +122,10 @@ TRACE_EVENT_FORMAT(print, TRACE_PRINT, print_entry, ignore, TRACE_EVENT_FORMAT(branch, TRACE_BRANCH, trace_branch, ignore, TRACE_STRUCT( TRACE_FIELD(unsigned int, line, line) - TRACE_FIELD_SPECIAL(char func[TRACE_FUNC_SIZE+1], func, - TRACE_FUNC_SIZE+1, func) - TRACE_FIELD_SPECIAL(char file[TRACE_FUNC_SIZE+1], file, - TRACE_FUNC_SIZE+1, file) + TRACE_FIELD_SPECIAL(char func[TRACE_FUNC_SIZE], func, + TRACE_FUNC_SIZE, func) + TRACE_FIELD_SPECIAL(char file[TRACE_FUNC_SIZE], file, + TRACE_FUNC_SIZE, file) TRACE_FIELD(char, correct, correct) ), TP_RAW_FMT("%u:%s:%s (%u)") ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] tracing: use strlcpy instead of strncpy 2009-05-19 1:51 ` Lai Jiangshan @ 2009-05-19 12:54 ` Stefan Richter 2009-05-20 2:34 ` Lai Jiangshan 2009-05-19 23:43 ` Frederic Weisbecker 1 sibling, 1 reply; 6+ messages in thread From: Stefan Richter @ 2009-05-19 12:54 UTC (permalink / raw) To: Lai Jiangshan; +Cc: Ingo Molnar, Jiri Slaby, Steven Rostedt, Li Zefan, LKML Lai Jiangshan wrote: > [Impact] cleanup Well, not entirely. > --- a/kernel/trace/trace.h > +++ b/kernel/trace/trace.h > @@ -155,8 +155,8 @@ struct trace_boot_ret { > struct trace_branch { > struct trace_entry ent; > unsigned line; > - char func[TRACE_FUNC_SIZE+1]; > - char file[TRACE_FILE_SIZE+1]; > + char func[TRACE_FUNC_SIZE]; > + char file[TRACE_FILE_SIZE]; > char correct; > }; (without change of the TRACE_????_SIZE definition) Better avoid a categorization of a patch if the change's scope exceeds that of the categorization. -- Stefan Richter -=====-=-=== -=-= -==-= http://arcgraph.de/sr/ ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] tracing: use strlcpy instead of strncpy 2009-05-19 12:54 ` Stefan Richter @ 2009-05-20 2:34 ` Lai Jiangshan 0 siblings, 0 replies; 6+ messages in thread From: Lai Jiangshan @ 2009-05-20 2:34 UTC (permalink / raw) To: Stefan Richter Cc: Ingo Molnar, Jiri Slaby, Steven Rostedt, Li Zefan, LKML, Frederic Weisbecker Stefan Richter wrote: > Lai Jiangshan wrote: >> [Impact] cleanup > > Well, not entirely. > >> --- a/kernel/trace/trace.h >> +++ b/kernel/trace/trace.h >> @@ -155,8 +155,8 @@ struct trace_boot_ret { >> struct trace_branch { >> struct trace_entry ent; >> unsigned line; >> - char func[TRACE_FUNC_SIZE+1]; >> - char file[TRACE_FILE_SIZE+1]; >> + char func[TRACE_FUNC_SIZE]; >> + char file[TRACE_FILE_SIZE]; >> char correct; >> }; > > (without change of the TRACE_????_SIZE definition) > > Better avoid a categorization of a patch if the change's scope exceeds > that of the categorization. I think it's make no sense to change the magic number TRACE_????_SIZE, I will rewrite my changelog as Frederic suggests. Thank you very much. Lai. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] tracing: use strlcpy instead of strncpy 2009-05-19 1:51 ` Lai Jiangshan 2009-05-19 12:54 ` Stefan Richter @ 2009-05-19 23:43 ` Frederic Weisbecker 1 sibling, 0 replies; 6+ messages in thread From: Frederic Weisbecker @ 2009-05-19 23:43 UTC (permalink / raw) To: Lai Jiangshan Cc: Ingo Molnar, Jiri Slaby, Steven Rostedt, Li Zefan, LKML, Stefan Richter On Tue, May 19, 2009 at 09:51:50AM +0800, Lai Jiangshan wrote: > strlcpy() will add '\0' for the copied string. > > [Impact] cleanup As Stefan said, the impact line and the changelog is omitting a possible side effect. > > Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com> > --- > diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c > index 05b4747..9359e85 100644 > --- a/kernel/trace/blktrace.c > +++ b/kernel/trace/blktrace.c > @@ -434,8 +434,7 @@ int do_blk_trace_setup(struct request_queue *q, char *name, dev_t dev, > if (!buts->buf_size || !buts->buf_nr) > return -EINVAL; > > - strncpy(buts->name, name, BLKTRACE_BDEV_SIZE); > - buts->name[BLKTRACE_BDEV_SIZE - 1] = '\0'; > + strlcpy(buts->name, name, BLKTRACE_BDEV_SIZE); > > /* > * some device names have larger paths - convert the slashes > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c > index 665a915..2484555 100644 > --- a/kernel/trace/trace.c > +++ b/kernel/trace/trace.c > @@ -133,7 +133,7 @@ static char *default_bootup_tracer; > > static int __init set_ftrace(char *str) > { > - strncpy(bootup_tracer_buf, str, BOOTUP_TRACER_SIZE); > + strlcpy(bootup_tracer_buf, str, BOOTUP_TRACER_SIZE); > default_bootup_tracer = bootup_tracer_buf; > /* We are using ftrace early, expand it */ > ring_buffer_expanded = 1; > diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h > index 6e735d4..e08c952 100644 > --- a/kernel/trace/trace.h > +++ b/kernel/trace/trace.h > @@ -155,8 +155,8 @@ struct trace_boot_ret { > struct trace_branch { > struct trace_entry ent; > unsigned line; > - char func[TRACE_FUNC_SIZE+1]; > - char file[TRACE_FILE_SIZE+1]; > + char func[TRACE_FUNC_SIZE]; > + char file[TRACE_FILE_SIZE]; Although I don't think this is that much risky, this is the possible side effect: we are loosing a character. I guess it's fine, the CPP constants here seem to be just magic assumptions (func_size = 30, file_size = 20), it just needs to be mentioned in the changelog for future possible bug tracking. Thanks, Frederic. > char correct; > }; > > diff --git a/kernel/trace/trace_branch.c b/kernel/trace/trace_branch.c > index 7a7a9fd..a84e036 100644 > --- a/kernel/trace/trace_branch.c > +++ b/kernel/trace/trace_branch.c > @@ -67,10 +67,8 @@ probe_likely_condition(struct ftrace_branch_data *f, int val, int expect) > p--; > p++; > > - strncpy(entry->func, f->func, TRACE_FUNC_SIZE); > - strncpy(entry->file, p, TRACE_FILE_SIZE); > - entry->func[TRACE_FUNC_SIZE] = 0; > - entry->file[TRACE_FILE_SIZE] = 0; > + strlcpy(entry->func, f->func, TRACE_FUNC_SIZE); > + strlcpy(entry->file, p, TRACE_FILE_SIZE); > entry->line = f->line; > entry->correct = val == expect; > > diff --git a/kernel/trace/trace_event_types.h b/kernel/trace/trace_event_types.h > index 5e32e37..2ae6679 100644 > --- a/kernel/trace/trace_event_types.h > +++ b/kernel/trace/trace_event_types.h > @@ -122,10 +122,10 @@ TRACE_EVENT_FORMAT(print, TRACE_PRINT, print_entry, ignore, > TRACE_EVENT_FORMAT(branch, TRACE_BRANCH, trace_branch, ignore, > TRACE_STRUCT( > TRACE_FIELD(unsigned int, line, line) > - TRACE_FIELD_SPECIAL(char func[TRACE_FUNC_SIZE+1], func, > - TRACE_FUNC_SIZE+1, func) > - TRACE_FIELD_SPECIAL(char file[TRACE_FUNC_SIZE+1], file, > - TRACE_FUNC_SIZE+1, file) > + TRACE_FIELD_SPECIAL(char func[TRACE_FUNC_SIZE], func, > + TRACE_FUNC_SIZE, func) > + TRACE_FIELD_SPECIAL(char file[TRACE_FUNC_SIZE], file, > + TRACE_FUNC_SIZE, file) > TRACE_FIELD(char, correct, correct) > ), > TP_RAW_FMT("%u:%s:%s (%u)") > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2009-05-20 2:37 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2009-05-18 11:35 [PATCH] tracing: use strlcpy instead of strncpy Lai Jiangshan 2009-05-18 20:55 ` Jiri Slaby 2009-05-19 1:51 ` Lai Jiangshan 2009-05-19 12:54 ` Stefan Richter 2009-05-20 2:34 ` Lai Jiangshan 2009-05-19 23:43 ` Frederic Weisbecker
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox