[MPlayer-dev-eng] [PATCH] ftp charset selection support

[Reimar Döffinger]

On Sat, Apr 19, 2008 at 11:18:42PM +0400, Andrew Savchenko wrote:
> --- mplayer_base/mp_msg.h	2008-03-08 02:46:16.000000000 +0300
> +++ mplayer/mp_msg.h	2008-04-19 21:47:21.000000000 +0400
> @@ -127,6 +127,21 @@
>  #   endif
>  #endif /* __GNUC__ */
>  
> -const char* filename_recode(const char* filename);
> +#ifdef USE_ICONV
> +#include <iconv.h>
> +
> +/* Recode filename from <tocode> to <fromcode> charset.
> + *
> + * *icnv -- iconv handler, unique for caller;
> + * *buf  -- buffer for recoded result, must be allocated outside,
> + *          but may be grown inside the function;
> + * *buf_size -- size of buffer to use. */
> +const char* mp_recode_to(iconv_t* const icnv, 
> +    const char* const filename, 
> +    const char* const tocode, const char* const fromcode,
> +    char** const buf, size_t* const buf_size);
> +#endif /* USE_ICONV */
> +
> +const char* filename_recode(const char* const filename);

I see now point in about half of those consts, they IMO just obfuscate
the code, and esp. the declaration of filename_recode should not change.

> --- mplayer_base/mp_msg.c	2008-04-12 20:24:22.000000000 +0400
> +++ mplayer/mp_msg.c	2008-04-19 21:46:20.000000000 +0400
> @@ -31,38 +31,74 @@
>  int mp_msg_module = 0;
>  #ifdef USE_ICONV
>  char *mp_msg_charset = NULL;
> -static char *old_charset = NULL;
> -static iconv_t msgiconv;
> -#endif
> -
> -const char* filename_recode(const char* filename)
> +char *old_charset = NULL;
> +iconv_t msgiconv;
> +iconv_t inv_msgiconv = (iconv_t)(-1);

Why removing the static? Why moving those variables out instead of
leaving it in the filename_recode function?

> +char *recoded_filename;
> +size_t recoded_filename_size = MSGSIZE_MAX;
> +
> +// recode filename from <tocode> to <fromcode> charset
> +const char* mp_recode_to(iconv_t* const icnv, 
> +    const char* const filename, 
> +    const char* const tocode, const char* const fromcode,
> +    char** const buf, size_t* const buf_size)
>  {
> -#if !defined(USE_ICONV) || !defined(MSG_CHARSET)
> -    return filename;
> -#else

And e.g. this (and a bit of other code) still exists after the patch, by
simply moving the new filename_recode function before the mp_recode_to
quite a bit of diff should disappear and make this much, much easier to
understand.