Re: [Nmh-workers] [patch] gcc warnings: possible use of uninitialized data

[Peter Maydell]

Peter Maydell wrote:
>I agree that these should be dealt with (so as a developer you
>can eventually choose to compile with -Werror);

I've just committed a set of patches which mean we now compile
without any gcc warnings, at least with the gcc I'm using.
I've also suppressed the irrelevant waffle from tsort about
loops in its input.

I also fixed a crash in fnext/fpriv if they are asked to work
on an empty folder list:
http://git.savannah.gnu.org/cgit/nmh.git/commit/?id=9530ace10

>(There's also two warnings from compiling the output of
>flex, which are irritating because they can be silenced
>but only by adding a flex-specific "%options noinput nounput"

I opted to fix this in a portable and simple way whose only
downside is an extra 200 odd bytes in the binary:
http://git.savannah.gnu.org/cgit/nmh.git/commit/?id=4dd01e6c6

The rest of this message is elaboration on what I did, which
you can safely skip.

>>diff -r 7e963436013a sbr/fmt_rfc2047.c
>>--- a/sbr/fmt_rfc2047.c       Fri Nov 05 10:56:54 2010 -0300
>>+++ b/sbr/fmt_rfc2047.c       Fri Nov 05 16:36:28 2010 -0300
>>@@ -239,6 +239,7 @@
>>              /* base64 */
>>              int c1, c2, c3, c4;
>> 
>>+             c1 = c2 = c3 = c4 = 0;
>>              pp = startofmime;
>>              while (pp < endofmime) {
>>                  /* 6 + 2 bits */
>
>Maybe these should be -1? Maybe this code for de-base64'ing
>could be replaced with something less cryptic instead :-)

I went for -1, although the variables can't actually be used
uninitialised.

>>diff -r 7e963436013a sbr/folder_addmsg.c
>>--- a/sbr/folder_addmsg.c     Fri Nov 05 10:56:54 2010 -0300
>>+++ b/sbr/folder_addmsg.c     Fri Nov 05 16:36:28 2010 -0300
>>@@ -22,7 +22,7 @@
>> folder_addmsg (struct msgs **mpp, char *msgfile, int selected,
>>                int unseen, int preserve, int deleting, char *from_dir)
>> {
>>-    int infd, outfd, linkerr, first_time, msgnum;
>>+    int infd, outfd, linkerr, first_time, msgnum = 0;
>>     char *nmsg, newmsg[BUFSIZ];
>>     char oldmsg[BUFSIZ];
>>     struct msgs *mp;
>
>This one might be better addressed by the following patch, which
>makes the code a bit easier to understand for humans and
>compilers both; although I haven't tested it beyond confirming
>that it does silence the warning :-)

I applied the patch I suggested.

>>diff -r 7e963436013a uip/inc.c
>>--- a/uip/inc.c       Fri Nov 05 10:56:54 2010 -0300
>>+++ b/uip/inc.c       Fri Nov 05 16:36:28 2010 -0300
>>@@ -236,7 +236,7 @@
>> {
>>     int chgflag = 1, trnflag = 1;
>>     int noisy = 1, width = 0;
>>-    int rpop, i, hghnum = 0, msgnum = 0;
>>+    int rpop, i = 0, hghnum = 0, msgnum = 0;
>>     int kpop = 0, sasl = 0;
>>     char *cp, *maildir = NULL, *folder = NULL;
>>     char *format = NULL, *form = NULL;
>
>NAK. This seems to be a rather large function which is using
>'i' as a "did we hit an error?" flag to see whether we should
>take an error-exit path at the end. 

This I haven't addressed (yet) because the compiler I'm using
doesn't complain about this function. I do have an untested
patch to address it lurking on another machine somewhere.

>>diff -r 7e963436013a uip/new.c
>>--- a/uip/new.c       Fri Nov 05 10:56:54 2010 -0300
>>+++ b/uip/new.c       Fri Nov 05 16:36:28 2010 -0300
>>@@ -297,7 +297,7 @@
>> static struct node *
>> doit(char *cur, char *folders, char *sequences[])
>> {
>>-    struct node *first, *cur_node, *node, *last, *prev;
>>+    struct node *first, *cur_node, *node, *last = NULL, *prev;
>>     size_t folder_len;
>>     int count, total = 0;
>>     char *command = NULL, *sequences_s = NULL;
>
>I think this one may be indicating a genuine bug where
>we don't do the right thing if check_folders() sets
>first to NULL. We should fix the bug instead.

I fixed the bug (see above) and the warning too.

-- PMM