List:Internals« Previous MessageNext Message »
From:Erik Ljungstrom Date:April 27 2009 11:15am
Subject:Re: [Fwd: your patch for BUG#40368 "mysqld_safe to honour underscore
same as dash on server options "]
View as plain text  
Hi Guilhem,

I've had a look at the comments below and reached the same conclusion as 
yourself, well spotted. I've tested your suggestion on the platforms I've 
got access to, and it works fine on those (none too obscure 
unfortunately). Thus I have no objections to your alteration.

Cheers,
--
Erik Ljungstrom
erik@stripped
http://northernmost.org/

On Mon, 27 Apr 2009, Guilhem Bichot wrote:

> Hi Erik,
>
> As you have seen, I'm reviewing your patch. I had previously sent my comments 
> to Arjen, so I'm forwarding them to you now. Could you please take a look at 
> them?
> When replying, please cc:internals@stripped.
> Thank you very much!
>
> -------- Message original --------
> Sujet: your patch for BUG#40368 "mysqld_safe to honour underscore same as 
> dash on server options "
> Date: Thu, 02 Apr 2009 14:57:10 +0200
> De: Guilhem Bichot <guilhem@stripped>
> Pour :: arjen@stripped
> Copie à :: internals <internals@stripped>
>
> Hi Arjen,
>
> I'm looking at your patch; agree with the bug's description and idea of
> the patch. Good that you insisted on it, we can be slow sometimes :)
> The patch is:
>
> --- mysql-5.0.67.orig/scripts/mysqld_safe.sh    2008-08-04
> 13:20:02.000000000 +0100
> +++ mysql-5.0.67/scripts/mysqld_safe.sh 2008-10-28 22:11:06.000000000 +0000
> @@ -59,6 +59,14 @@
>   fi
>
>   for arg do
> +
> +    # Replaces all underscores with dashes (excluding the two initial ones
> +    # and any instances after the first = character) since mysqld accepts
> +    # variables in this format.
> +    substr_a=`echo $arg | sed -e 's/\(...\)\(.*\)=\(.*\)/\2/'`
>
> Here, you eliminate the 3 first characters ("..."). So if arg is
> --open_files_limit=32, substr_a is "pen_files_limit". In the end it
> works, but why not eliminate only two (two dots)? Why use ".." and not
> "--" (What else can there be aprt from "--", in this position?).
>
> For the unlikely case where the user would do (gasp):
> --datadir=/some/path/with_=_in/it ,
> we wouldn't want to change "_" to "-" in the path. One would have to be
> crazy, but I prefer to avoid introducing a rare bug when fixing another.
> I propose two precautions to avoid that:
> - forcing the regex to match at start of string, only
> - forcing it to not be greedy by picking the first "="(see below):
>
> +    substr_b=`echo $substr_a | sed s/_/-/g`
> +    arg=`echo $arg | sed s/$substr_a/$substr_b/g`
>
> With the --datadir option value above, the substituation here fails
> because the sed command line expands to:
> sed s/atadir=/some/path/with_/atadir=/some/path/with-/g
> (at least with "bash" on my Linux laptop).
> I am thinking about this:
> substr_a=`echo $arg | sed -e 's/^\(..\)\([^=]*\)=\(.*\)/\2/'`
> though I haven't tested on <obscure platform>.
>
> Any ideas?
>
>
> -- 
> Mr. Guilhem Bichot <guilhem@stripped>
> Sun Microsystems / MySQL, Lead Software Engineer
> Bordeaux, France
> www.sun.com / www.mysql.com
>
Thread
Re: [Fwd: your patch for BUG#40368 "mysqld_safe to honour underscoresame as dash on server options "]Erik Ljungstrom27 Apr