Re: [PATCH] localedef: Add --localedir and --archive-file options (Bug 19130)

[Mike Frysinger <vapier at gentoo dot org>]

On 14 Oct 2015 23:52, Carlos O'Donell wrote:
> +    N_("Optional path to locale directory e.g. /usr/lib64/locale") },

why /usr/lib64/locale ?  seems like you should use LOCALEDIR.

> +    N_("locale-archive file to use instead of default")},

we do set up ARCHIVE_NAME which includes the full path ...

> +static const char *
> +construct_prefix_path (const char *prefix,
> +		       const char *localedir,
> +		       const char *path)
> +{
> +  ssize_t n;
> +  char *result;
> +  n = asprintf (&result, "%s%s%s%s%c",

asprintf returns an int, not ssize_t ... although i see the rest of this
program gets it wrong too ...

> +		prefix ?: "",
> +		localedir ?: "",
> +		localedir ? "/" : "",
> +		path, '\0');

what's with the \0 ?  asprintf does that for you ...
i guess the rest of this file does this weirdness too.  

> -  retval = (char *) malloc ((only_digit ? 3 : 0) + len + 1);
> +  retval = (char *) xmalloc ((only_digit ? 3 : 0) + len + 1);
>  
>    if (retval != NULL)

why check if retval is NULL since you changed it to xmalloc ?

> +#define ARCHIVE_NAME LOCALEDIR "/"LOCALE_ARCHIVE

should be a space after the "/"
-mike

Attachment: signature.asc
Description: Digital signature