[Freeciv-Dev] Re: (PR#2262) [Bug][Patch] Really call put_conv

[Vasco Alexandre Da Silva Costa <vasc@xxxxxxxxxxxxxx>]

On Sun, 3 Nov 2002, Raimar Falke via RT wrote:

> On Sun, Nov 03, 2002 at 10:36:21AM -0800, Vasco Alexandre da Silva Costa via 
> RT wrote:
> >
> > [vasc - Sun Nov  3 17:59:17 2002]:
>
> > Index: client/gui-gtk-2.0/gui_main.c
> > ===================================================================
> > RCS file: /home/freeciv/CVS/freeciv/client/gui-gtk-2.0/gui_main.c,v
> > retrieving revision 1.29
> > diff -u -r1.29 gui_main.c
> > --- client/gui-gtk-2.0/gui_main.c   2002/11/02 21:27:43     1.29
> > +++ client/gui-gtk-2.0/gui_main.c   2002/11/03 18:34:49
> > @@ -168,21 +168,24 @@
> >  /**************************************************************************
> >  ...
> >  **************************************************************************/
> > -static unsigned char *put_conv(unsigned char *dst, const char *src)
> > +static unsigned char *put_conv(const char *src, size_t *length)
> >  {
> >    gsize len;
>
> Why the extra variable?

Because I don't like passing pointers to variables of different types as
function arguments that's why. That function takes a gsize type argument
not size_t.

> > -  gchar *out = g_convert(src, -1, network_charset, "UTF-8", NULL, &len, 
> > NULL);
> > +  gchar *out =
> > +    g_convert(src, -1, network_charset, "UTF-8", NULL, &len, NULL);
> >
> >    if (out) {
> > +    unsigned char *dst = fc_malloc(len);
> > +
> >      memcpy(dst, out, len);
> >      g_free(out);
> >      dst[len] = '\0';
> >
> > -    return dst + len + 1;
> > +    *length = len;
> > +    return dst;
> >    } else {
> > -    dst[0] = '\0';
> > -
> > -    return dst + 1;
> > +    *length = 0;
> > +    return NULL;
> >    }
> >  }
> >
> > Index: common/dataio.c
> > ===================================================================
> > RCS file: /home/freeciv/CVS/freeciv/common/dataio.c,v
> > retrieving revision 1.4
> > diff -u -r1.4 dataio.c
> > --- common/dataio.c 2002/10/09 13:31:09     1.4
> > +++ common/dataio.c 2002/11/03 18:34:50
> > @@ -59,22 +59,11 @@
> >  /**************************************************************************
> >  ...
> >  **************************************************************************/
> > -static unsigned char *put_conv(unsigned char *dst, const char *src)
> > -{
> > -  size_t len = strlen(src) + 1;
> > -
> > -  memcpy(dst, src, len);
> > -  return dst + len;
> > -}
> > +static DIO_PUT_CONV_FUN put_conv_callback = NULL;
> >
> >  /**************************************************************************
> >  ...
> >  **************************************************************************/
> > -static DIO_PUT_CONV_FUN put_conv_callback = put_conv;
> > -
> > -/**************************************************************************
> > -...
> > -**************************************************************************/
> >  void dio_set_put_conv_callback(DIO_PUT_CONV_FUN fun)
> >  {
> >    put_conv_callback = fun;
> > @@ -329,7 +318,17 @@
> >  **************************************************************************/
> >  void dio_put_string(struct data_out *dout, const char *value)
> >  {
> > -  dio_put_memory(dout, value, strlen(value) + 1);
> > +  if (put_conv_callback) {
> > +    size_t length;
>
> > +    unsigned char *buffer;
> > +
> > +    if ((buffer = (*put_conv_callback) (value, &length))) {
>
> Merge.

Hmm?

> > +      dio_put_memory(dout, buffer, length + 1);
>
> Wrong. length already contains the whole length.

I guess you didn't test your own patch. Try removing the +1 and see what
happens... length doesn't count the terminating NUL character.

> Also the problem in my patch "don't handle NULL" isn't a problem since
> length is 0 and so the pointer doesn't matter.

Maybe for dio_put_memory, but not for free(buffer). Crash, bang,
segmentation fault.

---
Vasco Alexandre da Silva Costa @ Instituto Superior Tecnico, Lisboa