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.
