Messages in this thread | | | Date | Wed, 20 May 2009 01:43:14 +0200 | From | Frederic Weisbecker <> | Subject | Re: [PATCH] tracing: use strlcpy instead of strncpy |
| |
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/
| |