Complete.Org: Mailing Lists: Archives: freeciv-dev: May 2002:
[Freeciv-Dev] Re: (PR#1355)
Home

[Freeciv-Dev] Re: (PR#1355)

[Top] [All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
To: Vasco Alexandre Da Silva Costa <vasc@xxxxxxxxxxxxxx>
Cc: freeciv-dev@xxxxxxxxxxx, bugs@xxxxxxxxxxxxxxxxxxx
Subject: [Freeciv-Dev] Re: (PR#1355)
From: Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>
Date: Tue, 14 May 2002 21:26:31 +0200
Reply-to: rf13@xxxxxxxxxxxxxxxxxxxxxx

On Wed, May 08, 2002 at 09:07:17PM +0200, Raimar Falke wrote:
> On Wed, May 08, 2002 at 10:12:25AM -0700, Vasco Alexandre Da Silva Costa 
> wrote:
> > On Wed, 8 May 2002, Raimar Falke wrote:
> > 
> > > On Tue, May 07, 2002 at 05:10:36PM -0700, Vasco Alexandre Da Silva Costa 
> > > wrote:
> > > > 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.5
> > > > diff -u -r1.5 gui_main.c
> > > > --- client/gui-gtk-2.0/gui_main.c       2002/04/05 05:56:44     1.5
> > > > +++ client/gui-gtk-2.0/gui_main.c       2002/05/08 00:02:01
> > > > @@ -722,6 +722,43 @@
> > > >  }
> > > >  
> > > >  
> > > > /**************************************************************************
> > > > +...
> > > > +**************************************************************************/
> > > 
> > > > +static unsigned char *put_conv(unsigned char *dst, const char *src)
> > > > +static bool iget_conv(char *dst, int ndst, unsigned char *src, int 
> > > > nsrc)
> > > 
> > > Why is one if the arg unsigned char and the other a char?
> > 
> > To match the type of the arguments of put_string() and iget_string().
> 
> I have never noticed this. What about changing 
>   unsigned char *base;          /* start of packet */
>   unsigned char *ptr;           /* address of next data to pull off */
> to void pointers in the long run?
> 
> > > > +{
> > > > +  gchar *out;
> > > > +  gsize len;                   /* length to copy, not including null */
> > > > +  bool ret;
> > > > +
> > > > +  ret = FALSE;
> > > 
> > > Should be merged.
> > 
> > Like this? bool ret = FALSE; ? Ok.
> 
> Yes.
> 
> > > > +/* network string conversion */
> > > > +typedef unsigned char * (*PUT_FUN) (unsigned char *dst, const char 
> > > > *src);
> > > > +void put_set_callback(PUT_FUN fun);
> > > > +
> > > > +typedef bool (*IGET_FUN) (char *dst, int ndst, unsigned char *src, int 
> > > > nsrc);
> > > > +void iget_set_callback(IGET_FUN fun);
> > > 
> > > Can you add a _conv? So that it reads PUT_CONV_FUN and 
> > > set_put_conv_callback
> > 
> > I didn't do that because it would make those lines over 80 chars in
> > length and i don't like verbose names. If it matters that much to you i
> > have no problem with changing it.
> > 
> > > Replace some of the int with size_t.
> > 
> > I always retained the types used in the previous code. Which ones do you
> > want me to change to size_t?
> 
> Everything except argc and ps_len.

Updated patch attached. Changes:
 - adding one const
 - reverted the return value of iget_conv

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 "Premature optimization is the root of all evil."
    -- D. E. Knuth in "Structured Programming with go to Statements"

Attachment: utf8-2.diff
Description: Text document


[Prev in Thread] Current Thread [Next in Thread]