[Freeciv-Dev] Re: [Patch] General cleanup (splint)

[Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>]

On Sun, Feb 03, 2002 at 06:17:49PM -0600, Mike Kaufman wrote:
> On Sun, Feb 03, 2002 at 11:39:03PM +0100, Raimar Falke wrote:
> > 
> > I didn't want to review another patch so I try something new: splint
> > (www.splint.org). If the weak mode is restricted further the number of
> > warnings went down to something manageable. The attached patch is the
> > result to fix these:
> >  - unused return values: either check the result (fwrite), replace if
> >  with something other (client/options.c) or ignore the value
> >  - incomplete struct/array initializing
> >  - empty if block (ai/aicity.c)
> > 
> > Discoveries:
> >  - ugly things like
> > -GtkWidget *input_dialog_create(GtkWidget *parent, char *dialogname,
> > -                          char *text, char *postinputtest,
> > -                          void *ok_callback, gpointer ok_cli_data,
> > -                          void *cancel_callback, gpointer cancel_cli_data)
> > +GtkWidget *input_dialog_create(GtkWidget * parent, char *dialogname,
> > +                              char *text, char *postinputtest,
> > +                              GtkSignalFunc ok_callback,
> > +                              gpointer ok_cli_data,
> > +                              GtkSignalFunc cancel_callback,
> > +                              gpointer cancel_cli_data)
> > 
> >  - nobody is interested in the return values of adjust_workers,
> >  key_city_workers, alloc_nations, do_unit_goto
> >  - nobody outside client/gui-gtk/mapctrl.c is interested in adjust_workers
> >  - common/ioz.c was really wrong all the time. Why hasn't anybody noticed 
> > this?
> >  - send_packet_attribute_chunk was really wrong
> >  - nobody uses the return value of do_unit_goto. The return value has
> >  been introduced 2001/10/04. So dead code was added.
> >  - gui-xaw/menu.h:enum MenuID and gui-gtk/menu.c:enum MenuID look identical
> > 
> > IMHO this tool is nice and I will try it a second this if these
> > changes are in.
>  
> I think that this is a nice tool certainly, but I really dislike the
> patch. My main problem is the 450-odd additions of (void) to functions.

> Entirely unncessary in my opinion (granted, I did not look at every one,
> there could be one or two that are justified)

These changes have found the problem mentioned above. This and the
ability to discover more problem is IMHO justification enough. Just
thing of a more strict compiler.

> See my previous post about the code 'beautification' issues which I also
> have with this particular patch as well.

> I would like to take another look without these changes, but the other
> changes _appear_ fine.

Please do.

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 "I was dead ... but I'm better now."
    -- Capitain Sheridan in Babylon 5