On Fri, 2016-01-22 at 08:27 +0100, Matthias Klose wrote: > On 22.01.2016 06:09, Martin Michlmayr wrote: > > In terms of build failures, I reported 520 bugs to Debian. Most of them > > were new GCC errors or warnings (some packages use -Werror and many > > -Werror=format-security). > > > > Here are some of the most frequent errors see: > > [...] > Martin tagged these issues; https://wiki.debian.org/GCC6 has links with these > bug searches.
Thanks. The -Wmisleading-indentation ones can be seen at: https://bugs.debian.org/cgi-bin/pkgreport.cgi?tag=gcc-6-misleading-indentation;users=debian-...@lists.debian.org I looked through them all. I opened PR c/69415 to cover some possible issues with how we deal with one-liners, which a couple of these relate to; I don't know if we want to tweak our policy for one-liners. Other than that, I felt that we're in good shape here: I think the things we warned about were reasonable to warn about (but I have an obvious bias here). I'm writing up some notes for the website's "porting to gcc 6" page based on this. Detailed notes follow: ###################################################################### https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=811572 Package: pimd/litite: Looking at, https://github.com/troglobit/libite/blob/master/lite.h#L134 the code in question seems to be: static inline int fisslashdir(char *dir) { if (!dir) return 0; if (strlen (dir) > 0) return dir[strlen (dir) - 1] == '/'; return 0; } ###################################################################### https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=811573 Package: rustc > /<<BUILDDIR>>/rustc-1.5.0+dfsg1/src/rt/miniz.c: In function > 'tinfl_decompress': > /<<BUILDDIR>>/rustc-1.5.0+dfsg1/src/rt/miniz.c:578:47: error: statement is > indented as if it were guarded by... [-Werror=misleading-indentation] > for ( i = 0; i <= 143; ++i) *p++ = 8; for ( ; i <= 255; ++i) *p++ = > 9; for ( ; i <= 279; ++i) *p++ = 7; for ( ; i <= 287; ++i) *p++ = 8; > ^~~ Link to upstream rustc code in context: https://github.com/rust-lang/rust/blob/master/src/rt/miniz.c#L578 https://github.com/rust-lang/rust/blob/master/src/rt/miniz.c#L1396 My opinion: it's fair to issue a warning for this at -Wall ###################################################################### https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=811575 Package: nss The code in question seems to be here: https://hg.mozilla.org/projects/nss/file/tip/lib/dbm/src/h_page.c#l117 and contains mixed spaces and tabs. Detabifying at 8-spaces per tab (though am not sure if this is what upstream nss use), I get: if(origin == SEEK_CUR) { if(offset < 1) return(lseek(fd, offset, SEEK_CUR)); cur_pos = lseek(fd, 0, SEEK_CUR); if(cur_pos < 0) return(cur_pos); } IMHO the indentation here *is* misleading and hence the warning is justified. ###################################################################### https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=811576 Package: tpm-tools Code in question seems to be here: http://sources.debian.net/src/tpm-tools/1.3.8-2/src/tpm_mgmt/tpm_present.c/#L358 at the end of the function: out: if (szTpmPasswd && !isWellKnown) shredPasswd( szTpmPasswd ); return iRc; IMHO indentation here is somewhat misleading; warning feels justified ###################################################################### https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=811577 Package: sgt-puzzles Code in question seems to be here: https://github.com/radekp/sgt-puzzles/blob/master/towers.c#L391 if (ret) return ret; #ifdef STANDALONE_SOLVER if (solver_show_working) sprintf(prefix, "%*slower bounds for clue %s %d:\n", solver_recurse_depth*4, "", cluepos[c/w], c%w+1); else prefix[0] = '\0'; /* placate optimiser */ #endif Not sure how "misleading" this is, more just poorly indented. ###################################################################### https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=811578 Package: confetti Looks like it might be generated code e.g. generated by https://github.com/mailru/confetti/blob/master/c_dump.c ###################################################################### https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=811579 Package: mosh > make[5]: Entering directory '/<<PKGBUILDDIR>>/src/protobufs' ... > userinput.pb.cc: In member function 'virtual bool > ClientBuffers::Instruction::IsInitialized() const': > userinput.pb.cc:375:53: error: statement is indented as if it were guarded > by... [-Werror=misleading-indentation] > if (!_extensions_.IsInitialized()) return false; return true; > ^~~~~~ > > userinput.pb.cc:375:3: note: ...this 'if' clause, but it is not > if (!_extensions_.IsInitialized()) return false; return true; > ^~ Looks like it was autogenerated from: https://github.com/mobile-shell/mosh/blob/master/src/protobufs/userinput.proto presumably using https://github.com/google/protobuf (added to PR 69415) ###################################################################### https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=811580 Package: collectd Fixed upsptream: https://github.com/collectd/collectd/commit/376667a ###################################################################### https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=811582 Package: ion Code in question: http://sources.debian.net/src/ion/3.2.1%2Bdfsg-1/contrib/dtnperf/dtnperf/src/dtnperf_modes/dtnperf_client.c/#L670 and: http://sources.debian.net/src/ion/3.2.1%2Bdfsg-1/contrib/dtnperf/dtnperf/src/dtnperf_modes/dtnperf_client.c/#L719 Warnings look reasonable to me. ###################################################################### https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=811583 Package: modemmanager Code in question: https://sources.debian.net/src/modemmanager/1.4.12-1/libqcdm/tests/test-qcdm.c/#L46 https://sources.debian.net/src/modemmanager/1.4.12-1/libqcdm/tests/test-qcdm.c/#L52 Note the: /* -*- Mode: C; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */ at the top. Are these tabs? Yes. (this file is a violation of: https://developer.gnome.org/programming-guidelines/stable/c-coding-style.html.en which says: "Do not ever change the size of tabs in your editor; leave them as 8 spaces. Changing the size of tabs means that code that you didn’t write yourself will be perpetually misaligned." > gcc -DHAVE_CONFIG_H -I. -I../.. -pthread -I/usr/include/gio-unix-2.0/ > -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include > -I../../libqcdm/src -I../../src -Wdate-time -D_FORTIFY_SOURCE=2 -Wall > -Werror -std=gnu89 -g -O2 -fstack-protector-strong -Wformat > -Werror=format-security -Wmissing-declarations -Wmissing-prototypes > -Wdeclaration-after-statement -Wno-unused-parameter -Wno-sign-compare > -fno-strict-aliasing -Wno-deprecated-declarations > -Wno-unused-but-set-variable -Wformat-security -c -o test_qcdm-test-qcdm.o > `test -f 'test-qcdm.c' || echo './'`test-qcdm.c > test-qcdm.c: In function 'test_data_new': > test-qcdm.c:46:2: error: statement is indented as if it were guarded by... > [-Werror=misleading-indentation] > return d; > ^~~~~~ > > test-qcdm.c:43:5: note: ...this 'if' clause, but it is not > if (port) > ^~ > > test-qcdm.c: In function 'test_data_free': > test-qcdm.c:55:2: error: statement is indented as if it were guarded by... > [-Werror=misleading-indentation] > g_free (d); > ^~~~~~ > > test-qcdm.c:52:5: note: ...this 'if' clause, but it is not > if (d->com_data) > ^~ This bug has been fixed upstream in: http://cgit.freedesktop.org/ModemManager/ModemManager/commit/?id=99ae6777893d0b149cbefac3f290c63c47e29f42 The code in question is here: https://sources.debian.net/src/modemmanager/1.4.12-1/libqcdm/tests/test-qcdm.c/#L46 https://sources.debian.net/src/modemmanager/1.4.12-1/libqcdm/tests/test-qcdm.c/#L52 Note the: /* -*- Mode: C; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */ at the top. When viewed in Emacs, which respects the "tab-width: 4" option, the indentation does indeed reflect the block structure. But viewed via HTML in the links above, it looks wrong. ###################################################################### https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=811584 Package: phalanx Code in question is: https://sources.debian.net/src/phalanx/22%2Bd051004-13.1/bcreate.c/#L216 while( c!='[' && c!=EOF && Counter < MaxBookPly ) { while( (c=getchar()) != '.' && c!=EOF ) if(c=='{') { int csize=0; while( (c=getchar())!='}' && c!=EOF && csize<MaxComment ) csize++; } while( (c=getchar())==' ' || c=='\n' ) {} m[0]=c; i=1; while( (c=getchar())!=' ' && c!='\n' && c!=EOF ) { m[i]=c; i++; } m[i]='\0'; if( ! addmove(m) ) break; I think a warning is justified here. ###################################################################### https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=811585 Package: clutter-gesture https://sources.debian.net/src/clutter-gesture/0.0.2.1-7/clutter-gesture/clutter-gesture.c/#L715 https://sources.debian.net/src/clutter-gesture/0.0.2.1-7/clutter-gesture/clutter-gesture.c/#L743 couple of bad indentation of final "return" in fn. Not sure is misleading. Trivial to fix. ###################################################################### https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=811586 Package: bomberclone https://sources.debian.net/src/bomberclone/0.11.9-6/src/packets.c/#L1013 https://sources.debian.net/src/bomberclone/0.11.9-6/src/packets.c/#L1175 Warnings seem reasonable. ###################################################################### https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=811587 Package: libsoldout https://sources.debian.net/src/libsoldout/1.3-6/markdown.c/#L1284 Warning seems reasonable. ###################################################################### https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=811588 Package: grail Code in question: https://sources.debian.net/src/grail/3.1.0-2/tools/grail-test-atomic.c/#L148 quoting it here: if (status != UFStatusSuccess) fprintf(stderr, "Error: failed to get device from event\n"); else subscribe(grail_handle, device, subscribe_window, subscriptions, 2, 2); subscribe(grail_handle, device, subscribe_window, subscriptions, 3, 1); subscribe(grail_handle, device, subscribe_window, subscriptions, 4, 1); IMHO the warning is justified. ###################################################################### https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=811590 Package: rheolef Another one-liner: if (s.nop()) return s; s.os() << x; return s; What does it look like in context? e.g. http://www-ljk.imag.fr/membres/Pierre.Saramito/rheolef/source_html/diststream_8h_source.html ###################################################################### https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=811591 Package: ifhp Lots of one-liners e.g.: http://sources.debian.net/src/ifhp/3.5.20-13/src/ifhp.c/#L1348 (added to PR 69415) ###################################################################### https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=811592 Package: uwsgi Seems to be poor indentation: http://sources.debian.net/src/uwsgi/2.0.11.2-6/plugins/corerouter/cr_map.c/#L156 Warning feels justified. ###################################################################### https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=811593 Package: libgetopt++ http://sources.debian.net/src/libgetopt%2B%2B/0.0.2-p22-3.1/src/OptionSet.cc/#L126 http://sources.debian.net/src/libgetopt%2B%2B/0.0.2-p22-3.1/src/OptionSet.cc/#L159 Warnings feel justified. ###################################################################### https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=811594 Package: chordii http://sources.debian.net/src/chordii/4.3%2Brepack-2/src/a2crd.c/#L326 Strange indentation, throughout the file. Warning feels justified (at -Wall) ###################################################################### https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=811595 Package: upx-ucl > In file included from compress_lzma.cpp:237:0: > /usr/include/lzma/LzmaEnc.c: In function 'UInt32 GetOptimum(CLzmaEnc*, > UInt32, UInt32*)': > /usr/include/lzma/LzmaEnc.c:1350:9: warning: statement is indented as if it > were guarded by... [-Wmisleading-indentation] > { > ^ > > /usr/include/lzma/LzmaEnc.c:1346:7: note: ...this 'if' clause, but it is not > if (repIndex == 0) > ^~ Looks like this: https://github.com/moinakg/pcompress/blob/master/lzma/LzmaEnc.c#L1516 if (repIndex == 0) startLen = lenTest + 1; /* if (_maxMode) */ { UInt32 lenTest2 = lenTest + 1; ###################################################################### https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=811596 Package: librdkafka http://sources.debian.net/src/librdkafka/0.8.6-1.1/src/rdkafka.c/#L1144 if (rk->rk_conf.metadata_refresh_sparse) rd_kafka_broker_metadata_req(rkb, 0 /* known topics */, NULL, NULL, "sparse periodic refresh"); else rd_kafka_broker_metadata_req(rkb, 1 /* all topics */, NULL, NULL, "periodic refresh"); rd_kafka_broker_destroy(rkb); Warning feels justified. ###################################################################### https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=811598 Package: binutils-mingw-w64 > libtool: compile: gcc -DHAVE_CONFIG_H -I. -I../../../upstream/bfd -I. > -I../../../upstream/bfd -I../../../upstream/bfd/../include -DHAVE_i386_pe_vec > -DHAVE_i386_pei_vec -DHAVE_i386_elf32_vec -DHAVE_iamcu_elf32_vec > -DHAVE_elf32_le_vec -DHAVE_elf32_be_vec -DHAVE_plugin_vec > -DBINDIR=\"/usr/bin\" -Wdate-time -D_FORTIFY_SOURCE=2 -W -Wall > -Wstrict-prototypes -Wmissing-prototypes -Wshadow -Werror -g -O2 > -fstack-protector-strong -Wformat -Werror=format-security -c peigen.c -o > peigen.o > In file included from ../../../upstream/bfd/pe-i386.c:45:0: > ../../../upstream/bfd/coff-i386.c: In function 'coff_i386_reloc': > ../../../upstream/bfd/coff-i386.c:142:5: error: statement is indented as if > it were guarded by... [-Werror=misleading-indentation] > if (diff != 0) > ^~ > > ../../../upstream/bfd/coff-i386.c:133:3: note: ...this 'if' clause, but it is > not > if (reloc_entry->howto->type == R_IMAGEBASE > ^~ Fixed upstream: https://sourceware.org/ml/binutils/2015-12/msg00316.html ###################################################################### https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=811599 Package: libqmi Looks like another 4-space tab issue ###################################################################### https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=811600 Package: systemtap http://sources.debian.net/src/systemtap/2.9-2/tapsets.cxx/#L5007 Bad indentation of the brace. Easy fix. ###################################################################### https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=811602 Package: sword http://sources.debian.net/src/sword/1.7.3%2Bdfsg-7/src/keys/treekeyidx.cpp/#L262 Warning feels justified. ###################################################################### https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=811604 Package: crash Another copy of coff-i386.c ###################################################################### https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=811605 Package: gxneur http://sources.debian.net/src/gxneur/0.17.0-1/src/configuration.c/#L104 if(gcValue->type == GCONF_VALUE_STRING) g_free(*value), *value = g_strdup(gconf_value_get_string(gcValue)); result = 0; gconf_value_free(gcValue); Warning feels justified ###################################################################### https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=811606 Package: mongodb > src/mongo/client/dbclientinterface.h: In member function 'virtual void > mongo::DBConnector::checkResponse(const char*, int, bool*, > std::__cxx11::string*)': > src/mongo/client/dbclientinterface.h:524:41: error: statement is indented as > if it were guarded by... [-Werror=misleading-indentation] > if( retry ) *retry = false; if( targetHost ) *targetHost = ""; > ^~ Another one-liner. ###################################################################### https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=812137 Package: mailavenger http://sources.debian.net/src/mailavenger/0.8.4-4/libasync/maketables.c/#L135 Warning seems reasonable ###################################################################### https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=812166 Package: xen Seems to be: http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=xen/arch/x86/cpu/mcheck/non-fatal.c;h=526864ed339d1e2c8bd21d5a65218b64859dde7b;hb=HEAD#l97 and has apparently been fixed. ###################################################################### https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=811574 Package: glibc > In file included from ../stdlib/strtoul_l.c:27:0, > from ../sysdeps/wordsize-64/strtoul_l.c:6: > ../stdlib/strtol_l.c: In function '____strtoul_l_internal': > ../stdlib/strtol_l.c:360:9: error: statement is indented as if it were > guarded by... [-Werror=misleading-indentation] > cnt < thousands_len; }) > ^~~ > > ../stdlib/strtol_l.c:357:9: note: ...this 'for' clause, but it is not > && ({ for (cnt = 0; cnt < thousands_len; ++cnt) > ^~~ https://sourceware.org/git/?p=glibc.git;a=blob;f=stdlib/strtol_l.c;h=f4c17694ab39a2fd404b1d8c73466ac6bb1e4237;hb=HEAD#l353 ###################################################################### https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=811581 Package: putty (discussion in that bug; acceptance that the warning is reasonable: "I couldn't really make a clear argument for this case being part of any well defined space of things for which the warning ought not to trigger") ###################################################################### https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=811603 Package: cbmc "Thanks a lot for the heads-up, indenting has been cleaned up in CBMC trunk and compilation using GCC 6 appears to be fine in trunk. " ###################################################################### https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=811589 Package: netcfg http://sources.debian.net/src/netcfg/1.135/wpa.c/#L260 Fixed upstream ###################################################################### https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=811597 Package: linux-tools (the perf bug seen earlier)
