List:Commits« Previous MessageNext Message »
From:Guilhem Bichot Date:November 25 2010 9:35am
Subject:Re: bzr commit into mysql-trunk-bugfixing branch (mats.kindahl:3388)
Bug#58455
View as plain text  
Hello Mats,

Mats Kindahl a écrit, Le 24.11.2010 15:20:
> #At file:///home/bzr/mkindahl/testing-trunk-bugfixing/ based on
> revid:sergey.glukhov@stripped
> 
>  3388 Mats Kindahl	2010-11-24
>       Bug #58455
>       Starting mysqld with defaults file without
>       extension cause segmentation fault
>       
>       Bug occurs because fn_expand calls fn_format
>       with NULL as ext.

> === added file 'mysql-test/t/mysqld--defaults-file.test'
> --- a/mysql-test/t/mysqld--defaults-file.test	1970-01-01 00:00:00 +0000
> +++ b/mysql-test/t/mysqld--defaults-file.test	2010-11-24 14:20:25 +0000
> @@ -0,0 +1,31 @@
> +#
> +# mysqld --defaults-file
> +#
> +
> +--source include/not_embedded.inc
> +
> +# All these tests refer to configuration files that do not exist
> +
> +--error 1
> +--exec $MYSQLD --defaults-file=/path/with/no/extension --print-defaults 2>&1

I suggest writing in the test why MYSQLD is used instead of 
MYSQLD_BOOTSTRAP_CMD

> === modified file 'mysys/mf_format.c'
> --- a/mysys/mf_format.c	2009-04-19 01:21:33 +0000
> +++ b/mysys/mf_format.c	2010-11-24 14:20:25 +0000
> @@ -31,9 +31,12 @@ char * fn_format(char * to, const char *
>    reg1 size_t length;
>    size_t dev_length;
>    DBUG_ENTER("fn_format");
> +  DBUG_ASSERT(name != NULL);
> +  DBUG_ASSERT(extension != NULL);
>    DBUG_PRINT("enter",("name: %s  dir: %s  extension: %s  flag: %d",
>  		       name,dir,extension,flag));
>  
> +
>    /* Copy and skip directory */
>    name+=(length=dirname_part(dev, (startpos=(char *) name), &dev_length));
>    if (length == 0 || (flag & MY_REPLACE_DIR))

Two suggestions while you're in this function:
- add in the function comment of fn_expand(), that it allocates memory 
(which will thus have to be freed).
- "DBUG_ASSERT(result_path!=NULL)" could be removed, as it's preceded by
if (... (result_path=...)==NULL) DBUG_RETURN

Also, but it's not something to tackle here, there is something weird in 
the function's logic (but I prefer that we don't touch it (who knows 
what it would break?)):

   if (!(flag & MY_APPEND_EXT) &&
       (pos= (char*) strchr(name,FN_EXTCHAR)) != NullS)
   {
     if ((flag & MY_REPLACE_EXT) == 0)           /* If we should keep 
old ext */
     {
       length=strlength(name);                   /* Use old extension */
       ext = "";
     }
     else
     {
       length= (size_t) (pos-(char*) name);      /* Change extension */
       ext= extension;
     }
   }
   else
   {
     length=strlength(name);                     /* No ext, use the now 
one */
     ext=extension;
   }

If 'flags' does not contain MY_APPEND_EXT and 'name' does not contain an 
extension, we will come to the last "else" branch: it will add 
'extension' to the file's name. Even though we didn't pass MY_APPEND_EXT!
Fortunately, as you now pass "", we will add "", which is harmless.
Ok to push.
Thread
bzr commit into mysql-trunk-bugfixing branch (mats.kindahl:3388) Bug#58455Mats Kindahl24 Nov
Re: bzr commit into mysql-trunk-bugfixing branch (mats.kindahl:3388)Bug#58455Magnus BlĂ„udd24 Nov
  • Re: bzr commit into mysql-trunk-bugfixing branch (mats.kindahl:3388)Bug#58455Mats Kindahl24 Nov
Re: bzr commit into mysql-trunk-bugfixing branch (mats.kindahl:3388)Bug#58455Guilhem Bichot25 Nov
  • Re: bzr commit into mysql-trunk-bugfixing branch (mats.kindahl:3388)Bug#58455Guilhem Bichot25 Nov
  • Re: bzr commit into mysql-trunk-bugfixing branch (mats.kindahl:3388)Bug#58455Mats Kindahl26 Nov
  • Re: bzr commit into mysql-trunk-bugfixing branch (mats.kindahl:3388)Bug#58455Mats Kindahl26 Nov
Re: bzr commit into mysql-trunk-bugfixing branch(mats.kindahl:3388) Bug#58455Bjorn Munch25 Nov