| Message ID | 53C9191C.4070705@acm.org (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | Mike Snitzer |
| Headers | show |
Bart Van Assche wrote: > Inside libmultipath regcomp() is used to compile regular expressions > specified in /etc/multipath.conf. Many multipath.conf examples contain > 'product_type "*"'. However, "*" is not a valid POSIX regular expression. > Hence this patch that changes the regular expression "*" into ".*". > > Signed-off-by: Bart Van Assche <bvanassche@acm.org> > --- > libmultipath/blacklist.c | 15 +++++++++++---- > 1 file changed, 11 insertions(+), 4 deletions(-) > > diff --git a/libmultipath/blacklist.c b/libmultipath/blacklist.c > index 651bd7e..e5c287e 100644 > --- a/libmultipath/blacklist.c > +++ b/libmultipath/blacklist.c > @@ -63,6 +63,13 @@ alloc_ble_device (vector blist) > return 0; > } > > +static int lm_regcomp(regex_t *preg, const char *pattern, int cflags) > +{ > + if (strcmp(pattern, "*") == 0) > + pattern = ".*"; > + return regcomp(preg, pattern, cflags); > +} > + > extern int > set_ble_device (vector blist, char * vendor, char * product, int origin) > { > @@ -77,16 +84,16 @@ set_ble_device (vector blist, char * vendor, char * product, int origin) > return 1; > > if (vendor) { > - if (regcomp(&ble->vendor_reg, vendor, > - REG_EXTENDED|REG_NOSUB)) { > + if (lm_regcomp(&ble->vendor_reg, vendor, > + REG_EXTENDED|REG_NOSUB)) { > FREE(vendor); > return 1; > } > ble->vendor = vendor; > } > if (product) { > - if (regcomp(&ble->product_reg, product, > - REG_EXTENDED|REG_NOSUB)) { > + if (lm_regcomp(&ble->product_reg, product, > + REG_EXTENDED|REG_NOSUB)) { > FREE(product); > return 1; > } Is this change really required? With patch 4 we now get a proper error: multipath.conf +14 parsing failed: vendor "*" multipath.conf +15 parsing failed: product "*" error parsing config file I think it should be enough to modify the man page to mention vendor/product are both regular expressions. This change might also confuse users since this automagic "*" to ".*" only applies to the blacklist exceptions. Sebastian -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
On Thu, Jul 24, 2014 at 04:55:59PM +0200, Sebastian Herbszt wrote: > Bart Van Assche wrote: > > Inside libmultipath regcomp() is used to compile regular expressions > > specified in /etc/multipath.conf. Many multipath.conf examples contain > > 'product_type "*"'. However, "*" is not a valid POSIX regular expression. > > Hence this patch that changes the regular expression "*" into ".*". > > > Is this change really required? With patch 4 we now get a proper error: > multipath.conf +14 parsing failed: vendor "*" > multipath.conf +15 parsing failed: product "*" > error parsing config file > > I think it should be enough to modify the man page to mention vendor/product > are both regular expressions. This change might also confuse users since this > automagic "*" to ".*" only applies to the blacklist exceptions. Agreed; the examples are broken here so let's fix them rather than add magic parsing that will only come back to confuse people in the future. Regards, Bryn. -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Right, I agree with your comments. The patch is reverted. Thanks. On Thu, Jul 24, 2014 at 6:34 PM, Bryn M. Reeves <bmr@redhat.com> wrote: > On Thu, Jul 24, 2014 at 04:55:59PM +0200, Sebastian Herbszt wrote: > > Bart Van Assche wrote: > > > Inside libmultipath regcomp() is used to compile regular expressions > > > specified in /etc/multipath.conf. Many multipath.conf examples contain > > > 'product_type "*"'. However, "*" is not a valid POSIX regular > expression. > > > Hence this patch that changes the regular expression "*" into ".*". > > > > > Is this change really required? With patch 4 we now get a proper error: > > multipath.conf +14 parsing failed: vendor "*" > > multipath.conf +15 parsing failed: product "*" > > error parsing config file > > > > I think it should be enough to modify the man page to mention > vendor/product > > are both regular expressions. This change might also confuse users since > this > > automagic "*" to ".*" only applies to the blacklist exceptions. > > Agreed; the examples are broken here so let's fix them rather than add > magic > parsing that will only come back to confuse people in the future. > > Regards, > Bryn. > > -- > dm-devel mailing list > dm-devel@redhat.com > https://www.redhat.com/mailman/listinfo/dm-devel > -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
diff --git a/libmultipath/blacklist.c b/libmultipath/blacklist.c index 651bd7e..e5c287e 100644 --- a/libmultipath/blacklist.c +++ b/libmultipath/blacklist.c @@ -63,6 +63,13 @@ alloc_ble_device (vector blist) return 0; } +static int lm_regcomp(regex_t *preg, const char *pattern, int cflags) +{ + if (strcmp(pattern, "*") == 0) + pattern = ".*"; + return regcomp(preg, pattern, cflags); +} + extern int set_ble_device (vector blist, char * vendor, char * product, int origin) { @@ -77,16 +84,16 @@ set_ble_device (vector blist, char * vendor, char * product, int origin) return 1; if (vendor) { - if (regcomp(&ble->vendor_reg, vendor, - REG_EXTENDED|REG_NOSUB)) { + if (lm_regcomp(&ble->vendor_reg, vendor, + REG_EXTENDED|REG_NOSUB)) { FREE(vendor); return 1; } ble->vendor = vendor; } if (product) { - if (regcomp(&ble->product_reg, product, - REG_EXTENDED|REG_NOSUB)) { + if (lm_regcomp(&ble->product_reg, product, + REG_EXTENDED|REG_NOSUB)) { FREE(product); return 1; }
Inside libmultipath regcomp() is used to compile regular expressions specified in /etc/multipath.conf. Many multipath.conf examples contain 'product_type "*"'. However, "*" is not a valid POSIX regular expression. Hence this patch that changes the regular expression "*" into ".*". Signed-off-by: Bart Van Assche <bvanassche@acm.org> --- libmultipath/blacklist.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-)