| Message ID | 20230125062814.2517900-1-computersforpeace@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [fstools] partname: Correct fstools_partname_fallback_scan comparison | expand |
On Tue, Jan 24, 2023 at 10:28:14PM -0800, Brian Norris wrote: > We want to return NULL if the param does *not* match 1 -- i.e., a > non-zero strcmp(). > > Fixes: 1ea5855e980c ("partname: Introduce fstools_partname_fallback_scan option") Hmm, sorry for the quick self-reply, but after rereading, I noticed there's a second reason commit 1ea5855e980c may be incorrect: This fallback *used* to (pre-1ea5855e980c) be used for any block device (eMMC, SATA, etc.) where we *didn't* specify root= in the boot args. This could perhaps happen with initramfs systems? (Sorry, I'm not extremely familiar with the openwrt ecosystem.) So do we need to refactor this again, so that we allow the fallback in either (or both) of these cases: (1) no root= arg (2) root=XXXX (where XXX is not a /device/path) + fstools_partname_fallback_scan=1 ? Anyway, it's probably at least safe to apply my bugfix, but we might need more. Brian > Signed-off-by: Brian Norris <computersforpeace@gmail.com> > --- > > libfstools/partname.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libfstools/partname.c b/libfstools/partname.c > index f42322a49d5b..82c723c02097 100644 > --- a/libfstools/partname.c > +++ b/libfstools/partname.c > @@ -143,7 +143,7 @@ static struct volume *partname_volume_find(char *name) > if (!get_var_from_file("/proc/cmdline", "fstools_partname_fallback_scan", rootparam, sizeof(rootparam))) > return NULL; > > - if (!strcmp("1", rootparam)) > + if (strcmp("1", rootparam)) > return NULL; > > /* no useful 'root=' kernel cmdline parameter, find on any block device */ > -- > 2.39.0 >
On Tue, Jan 24, 2023 at 10:40:26PM -0800, Brian Norris wrote: > On Tue, Jan 24, 2023 at 10:28:14PM -0800, Brian Norris wrote: > > We want to return NULL if the param does *not* match 1 -- i.e., a > > non-zero strcmp(). > > > > Fixes: 1ea5855e980c ("partname: Introduce fstools_partname_fallback_scan option") > > Hmm, sorry for the quick self-reply, but after rereading, I noticed > there's a second reason commit 1ea5855e980c may be incorrect: > > This fallback *used* to (pre-1ea5855e980c) be used for any block device > (eMMC, SATA, etc.) where we *didn't* specify root= in the boot args. > This could perhaps happen with initramfs systems? (Sorry, I'm not > extremely familiar with the openwrt ecosystem.) > > So do we need to refactor this again, so that we allow the fallback in > either (or both) of these cases: > > (1) no root= arg > (2) root=XXXX (where XXX is not a /device/path) + > fstools_partname_fallback_scan=1 > > ? > > Anyway, it's probably at least safe to apply my bugfix, but we might > need more. > Hi, you are right but in theory it should not be that hard to rework... We surely need to handle the case with no root arg. > > Signed-off-by: Brian Norris <computersforpeace@gmail.com> > > --- > > > > libfstools/partname.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/libfstools/partname.c b/libfstools/partname.c > > index f42322a49d5b..82c723c02097 100644 > > --- a/libfstools/partname.c > > +++ b/libfstools/partname.c > > @@ -143,7 +143,7 @@ static struct volume *partname_volume_find(char *name) > > if (!get_var_from_file("/proc/cmdline", "fstools_partname_fallback_scan", rootparam, sizeof(rootparam))) > > return NULL; > > > > - if (!strcmp("1", rootparam)) > > + if (strcmp("1", rootparam)) > > return NULL; > > > > /* no useful 'root=' kernel cmdline parameter, find on any block device */ > > -- > > 2.39.0 > >
diff --git a/libfstools/partname.c b/libfstools/partname.c index f42322a49d5b..82c723c02097 100644 --- a/libfstools/partname.c +++ b/libfstools/partname.c @@ -143,7 +143,7 @@ static struct volume *partname_volume_find(char *name) if (!get_var_from_file("/proc/cmdline", "fstools_partname_fallback_scan", rootparam, sizeof(rootparam))) return NULL; - if (!strcmp("1", rootparam)) + if (strcmp("1", rootparam)) return NULL; /* no useful 'root=' kernel cmdline parameter, find on any block device */
We want to return NULL if the param does *not* match 1 -- i.e., a non-zero strcmp(). Fixes: 1ea5855e980c ("partname: Introduce fstools_partname_fallback_scan option") Signed-off-by: Brian Norris <computersforpeace@gmail.com> --- libfstools/partname.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)