From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from scc-mailout-kit-02.scc.kit.edu (scc-mailout-kit-02.scc.kit.edu [129.13.231.82]) by fantadrom.bsd.lv (OpenSMTPD) with ESMTP id 14173177 for ; Mon, 3 Jun 2019 15:37:17 -0500 (EST) Received: from asta-nat.asta.uni-karlsruhe.de ([172.22.63.82] helo=hekate.usta.de) by scc-mailout-kit-02.scc.kit.edu with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (envelope-from ) id 1hXthj-0002sJ-W5; Mon, 03 Jun 2019 22:37:17 +0200 Received: from donnerwolke.usta.de ([172.24.96.3]) by hekate.usta.de with esmtp (Exim 4.77) (envelope-from ) id 1hXthj-0005qu-0k; Mon, 03 Jun 2019 22:37:15 +0200 Received: from athene.usta.de ([172.24.96.10]) by donnerwolke.usta.de with esmtp (Exim 4.84_2) (envelope-from ) id 1hXthi-00009a-Ty; Mon, 03 Jun 2019 22:37:14 +0200 Received: from localhost (athene.usta.de [local]) by athene.usta.de (OpenSMTPD) with ESMTPA id 2d29a7ed; Mon, 3 Jun 2019 22:37:14 +0200 (CEST) Date: Mon, 3 Jun 2019 22:37:14 +0200 From: Ingo Schwarze To: Michal Nowak Cc: discuss@mandoc.bsd.lv Subject: Re: error: 'XXX' may be used uninitialized in this function Message-ID: <20190603203714.GA59058@athene.usta.de> References: <5a8f2439-c566-c9e2-f796-86dccbd697f9@startmail.com> X-Mailinglist: mandoc-discuss Reply-To: discuss@mandoc.bsd.lv MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <5a8f2439-c566-c9e2-f796-86dccbd697f9@startmail.com> User-Agent: Mutt/1.8.0 (2017-02-23) Hi Michael, Michal Nowak wrote on Mon, Jun 03, 2019 at 05:28:28PM +0200: > I work on updating mandoc to 1.14.5 on illumos Appreciated, thank you! > and there are few errors (warnings treated as errors) when building > mandoc with GCC 4.4.4 and 7.4.0: [text reordered] > In all cases I was able to eliminate those errors by initializing > respective variables to NULL. In general, i object to blindly initializing variables without considering whether it makes sense logically; that is dangerous behaviour on part of the programmer because it can hide errors in the code - both existing errors and errors introduced later. > It seems that none of those errors were > addressed after the 1.14.5 release. No, because no are errors: they are all false positives. > In function 'print_mdoc_node': > error: 'act' may be used uninitialized in this function > [-Werror=maybe-uninitialized] > if (act->post == NULL || n->flags & NODE_ENDED) > ~~~^~~~~~ The pointer act is assigned to in the default case of the previous n->type switch, and it is used only in the default case of the second n->type switch. That said, the only way this pointer is used is by dereferencing it. So (pointlessly) initializing it does no harm because in case a bug is introduced later, it changes an uninitialized pointer access into a NULL pointer access which is potentially less dangerous. So in this particular case, your suggestion to hide the error is OK. > In function 'mparse_buf_r': > /export/home/newman/ws/oi-userland/components/openindiana/illumos-gate/illumos-gate/usr/src/cmd/mandoc/read.c:147: > error: 'lastln' may be used uninitialized in this function [-Wuninitialized] Another false positive: lastln is only used when firstln is already set, and whenever firstln gets set, lastln is also set at the same time. That said, it is logically clear from the purpose of the variable that initializing lastln is harmless in this case. Logically, lastln *should* be NULL as long as firstln is NULL. > In function 'term_fill': > /export/home/newman/ws/oi-userland/components/openindiana/illumos-gate/illumos-gate/usr/src/cmd/mandoc/term.c:286:24: > error: 'vn' may be used uninitialized in this function > [-Werror=maybe-uninitialized] > if (breakline || vn > vtarget) And yet another false positive. The variable vn is assigned to in all three cases of the inner switch and only used immediately afterwards. Apparently, the compiler isn't smart enough to see that the cases of the inner switch are exhaustive. Then again, there is no harm in saying so explicitly. So, i committed the three patches shown below. Can you confirm that your gcc no longer complains? Yours, Ingo Log Message: ----------- Initialize the local variable "act" in print_mdoc_node(). While there is no bug, it helps clarity, and it is also safer in this particular code because in case a bug gets introduced later, accessing a NULL pointer is less dangerous than accessing an uninitialized pointer. Michal Nowak reported that gcc 4.4.4 and 7.4.0 on illumos throw -Wuninitialized false positives. Modified Files: -------------- mandoc: mdoc_term.c Revision Data ------------- Index: mdoc_term.c =================================================================== RCS file: /home/cvs/mandoc/mandoc/mdoc_term.c,v retrieving revision 1.372 retrieving revision 1.373 diff -Lmdoc_term.c -Lmdoc_term.c -u -p -r1.372 -r1.373 --- mdoc_term.c +++ mdoc_term.c @@ -352,6 +352,7 @@ print_mdoc_node(DECL_ARGS) * produce output. Note that some pre-handlers do so. */ + act = NULL; switch (n->type) { case ROFFT_TEXT: if (n->flags & NODE_LINE) { Log Message: ----------- Initialize the local variable "lastln" in mparse_buf_r(). While there is no bug, it logically makes sense given the meaning of the variable that lastln is NULL as long as firstln is NULL. Michal Nowak reported that gcc 4.4.4 and 7.4.0 on illumos throw -Wuninitialized false positives. Modified Files: -------------- mandoc: read.c Revision Data ------------- Index: read.c =================================================================== RCS file: /home/cvs/mandoc/mandoc/read.c,v retrieving revision 1.212 retrieving revision 1.213 diff -Lread.c -Lread.c -u -p -r1.212 -r1.213 --- read.c +++ read.c @@ -157,7 +157,7 @@ mparse_buf_r(struct mparse *curp, struct ln.sz = 256; ln.buf = mandoc_malloc(ln.sz); ln.next = NULL; - firstln = loop = NULL; + firstln = lastln = loop = NULL; lnn = curp->line; pos = 0; inloop = 0; Log Message: ----------- Initialize the local variable "lastln" in mparse_buf_r(). While there is no bug, it logically makes sense given the meaning of the variable that lastln is NULL as long as firstln is NULL. Michal Nowak reported that gcc 4.4.4 and 7.4.0 on illumos throw -Wuninitialized false positives. Modified Files: -------------- mandoc: read.c Revision Data ------------- Index: read.c =================================================================== RCS file: /home/cvs/mandoc/mandoc/read.c,v retrieving revision 1.212 retrieving revision 1.213 diff -Lread.c -Lread.c -u -p -r1.212 -r1.213 --- read.c +++ read.c @@ -157,7 +157,7 @@ mparse_buf_r(struct mparse *curp, struct ln.sz = 256; ln.buf = mandoc_malloc(ln.sz); ln.next = NULL; - firstln = loop = NULL; + firstln = lastln = loop = NULL; lnn = curp->line; pos = 0; inloop = 0; -- To unsubscribe send an email to discuss+unsubscribe@mandoc.bsd.lv