On Mon, Oct 28, 2013 at 01:06:45PM +0800, Dave Young wrote: > On 10/28/13 at 01:20pm, Simon Horman wrote: > > On Mon, Oct 28, 2013 at 10:30:32AM +0800, Dave Young wrote: > > > On 10/28/13 at 09:13am, Dave Young wrote: > > > > > > @@ -447,7 +446,7 @@ void setup_subarch(struct x86_linux_para > > > > > > if (!debugfs_mnt) > > > > > > return; > > > > > > snprintf(filename, PATH_MAX, "%s/%s", debugfs_mnt, "boot_params/data"); > > > > > > - filename[PATH_MAX-1] = 0; > > > > > > + filename[PATH_MAX - 1] = 0; > > > > > > free(debugfs_mnt); > > > > > > > > > > This change appears to be unrelated to the rest of the patch. > > > > > > > > > > > > > Will remove the change from the patch. > > > > > > Relooking it, with this patch the line "filename[PATH_MAX - 1] = 0;" > > > is in the share function get_bootparam, it's not in setup_subarch > > > any more so I think it's ok, what do you think? > > > > I'm a bit confused. > > > > My reading of the hunk above is that it is a whitespace-only change. > > If so it is not a change that I object to but also not one that > > I feel belongs in this patch. If not I am somehow mistaken. > > The diff context is cheating us, it might because of the context (+- 3 lines) > is same with original setup_arch, but it is really moving to a new function. Ok, in that case I have no objections.