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

[Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>]

On Mon, Feb 04, 2002 at 03:35:20AM -0500, Jason Short wrote:
> Raimar Falke wrote:
> 
> > On Mon, Feb 04, 2002 at 01:08:58AM -0500, Jason Short wrote:
> > 
> >>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)
> >>>
> >>I partially disagree.  In some cases, a return value on a function is 
> >>really not meant to be ignored, and ignoring it is a potentially large 
> >>bug.  
> > 
> >>This is often done with normalize_map_pos(), for instance. 
> > 
> > Interestingly I haven't found such a case.
> 
> And that's a good thing :-).  Most such places have been converted to
>    is_real = normalize_map_pos(...);
>    assert(is_real);

I played some more with splint and enabled some more strict mode. It
turns out that this produce some more unused return value warnings. So
there is no all-clear yet. For example my_snprintf's result is unused
a lot. I'm not sure how this should be handled. Add a lot of (void) or
change the return type to void.

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 "Understanding is a three-edged sword; 
  your side, their side, and the truth."
    -- a well-known Vorlon