List:Internals« Previous MessageNext Message »
From:Masood Mortazavi Date:April 17 2009 3:03am
Subject:Re: your patch for BUG#40368 "mysqld_safe to honour underscore same
as dash on server options "
View as plain text  
Guilhem and Arjen -

On Thu, Apr 9, 2009 at 3:31 AM, Arjen Lentz <arjen@stripped> wrote:
> Hi Guilhem
>
> On 02/04/2009, at 10:57 PM, Guilhem Bichot wrote:
>>
>> 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 :)
>
> I'm happy that you picked it up.
>
>
>> 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?
>
> Sounds sensible.
>
>
>> You mentioned fixing my_print_defaults, why is it needed?
>
> It was another approach to resolving the same issue, if I remember
> correctly.
>
>
>> By the way - paperwork. You have already signed the CLA in 2007; you will
>> be happy to know that there is more to do (somehow contribution agreements
>> have been revised since you signed):
>> http://forge.mysql.com/wiki/Contributing_Code#Paperwork
>
> Well, two things:
>  - Erik Ljungstrom did the patch, not I (he's cc'd on this email).
>  - This is a few lines of script/code, I think you are quite fine just using
> it. Particularly since you just rewrote it! From past experience at MySQL,
> we never insisted on the CLA for very small patches.
>
>
> Regards,
> Arjen.



Erik has just signed the SCA.
... and his name is already posted on the SCA signatories list:
https://sca.dev.java.net/CA_signatories.htm ...
That takes care of the paper work ...

- m.
Thread
your patch for BUG#40368"mysqld_safe to honour underscore same as dash on server options "Guilhem Bichot2 Apr
  • Re: your patch for BUG#40368 "mysqld_safe to honour underscore same as dash on server options "Arjen Lentz9 Apr
    • Re: your patch for BUG#40368 "mysqld_safe to honour underscore same as dash on server options "Masood Mortazavi17 Apr