Re: [Groff] problem with preconv and sample_docs.mom

[Ralph Corderoy]

Hi Branden,

>    if (debug_flag)
>      fprintf(stderr, "  charset: %s\n", charset);
>    if (charset) {
> +    /* uchardet 0.0.1 could return an empty string instead of NULL */
> +    if (!charset[0]) {
> +      uchardet_delete(ud);
> +      return NULL;
> +    }
>      ret = (char *)calloc(strlen(charset) + 1, 1);
>      strcpy(ret, charset);
>    }

That doesn't free(data), as the code continues to do after `if (charset)
{...}'.  If the test for charset being non-NULL isn't sufficient
then I'd expect just that to need changing.

    if (charset && *charset) {

But reading
http://git.savannah.gnu.org/cgit/groff.git/tree/src/preproc/preconv/preconv.cpp
suggests other problems, ignoring the lack of error checking.

  1004  char *
  1005  detect_file_encoding(FILE *fp)
  1006  {
  1007  #ifdef HAVE_UCHARDET
   ...
  1014      char *ret;
   ...
  1049      charset = uchardet_get_charset(ud);
  1050      if (debug_flag)
  1051          fprintf(stderr, "  charset: %s\n", charset);
  1052      if (charset) {
  1053          ret = (char *)calloc(strlen(charset) + 1, 1);
  1054          strcpy(ret, charset);
  1055      }
  1056      uchardet_delete(ud);
  1057      free(data);
  1058  
  1059      return ret;
  1060  #else /* not HAVE_UCHARDET */
  1061      return NULL;
  1062  #endif /* not HAVE_UCHARDET */
  1063  }

1052 suggests charset may be NULL but it's already been fprintf'd on the
previous line.

1059 returns an uninitialised value if charset is NULL since ret is only
set once, conditionally, at 1053.

-- 
Cheers, Ralph.
https://plus.google.com/+RalphCorderoy