Re: [tor-bugs] #19180 [Core Tor/Tor]: Add new compiler warnings

["Tor Bug Tracker & Wiki" <blackhole@xxxxxxxxxxxxxx>]

#19180: Add new compiler warnings
--------------------------+------------------------------
 Reporter:  nickm         |          Owner:  nickm
     Type:  defect        |         Status:  merge_ready
 Priority:  Medium        |      Milestone:  Tor: 0.2.???
Component:  Core Tor/Tor  |        Version:
 Severity:  Normal        |     Resolution:
 Keywords:  029-proposed  |  Actual Points:
Parent ID:                |         Points:
 Reviewer:                |        Sponsor:
--------------------------+------------------------------
Changes (by andrea):

 * status:  needs_review => merge_ready


Comment:

 Begin review of nickm's bug19180_easy branch:

 da00395318ef0d762b0d9affb494101a72f3b3ab:

  - This looks fine, but I haven't got a gcc 6 handy to test with; I
 presume
    that testing for this branch including making sure the code builds
 cleanly
    with new warnings enabled?

    (I see that other commits in this series also include fixes to make the
    code clean under the relevant warnings, so I suppose that's a yes)

 9bba1707c62170a89af8becbdda6c6e47be215a3:

  - This looks okay to me

 22b7bdd670cfeba6c5afcb6e177d27de45943ea6:

  - This looks good

 54610f721d66c41280e64674c27869eca381f059:

  - This looks fine to me given the subsequent fixup in
    bf1d9c725cb45df1eb7b0a4be57ae9da36763432

 4d9c67ff2e202ff9bcc171701cf73527e833f0ed:

  - Strong agreement about huge string constants in the unit tests; there's
 lots
    of places where not having them would make the unit tests much hairier;
 we'd
    end up either having to read them from a file or construct them at run
 time
    from smaller string constants.

  - Is there any way to make enabling/disabling the warning contingent on
 its
    existence rather than on the gcc version?  This seems really brittle if
 we
    ever change which gcc versions a particular warning is enabled by
 default
    for.

 94fb4f94305dd725176c99b7276fc8cbb133865e:

  - Looks good, and like it also fixes a bug setting have_gcc5 = no rather
 than
    have_gcc6 = no.

 bf1d9c725cb45df1eb7b0a4be57ae9da36763432:

  - Looks good to me

 a74f3abc52ded6cb69b9272d8dac3f22528bd98d:

  - The commit message suggests this addresses the point I raised in regard
    to 4d9c67ff2e202ff9bcc171701cf73527e833f0ed, but it actually only has a
    changes file.

 d979da8b017989e4aa053396b4ca37bd7deb980d:

  - This actually does what a74f3abc52ded6cb69b9272d8dac3f22528bd98d says;
    looks good

 1ee7cd459257bcc2cce614a65cd861f5ff8ea5aa:

  - Good catch

 7912aff13b464e229eeacbe5805356a4ffa2d66a:

  - Are we worried about the possibility of something like clang and gcc
    adding the same warning name for different behavior?

 2ab4bb35feafb1ef0aab9cb55e8201ce31ab9a1c:

  - Looks okay to me.

 ae82061f62bb7b9558ce81edb3271e000931b188:

  - Looks good

 aea2e38cea967415ec170dd5c5aacef02c113bce:

  - That's quite a diff.  I think it's all fine though.

 7f8c6723ba9c66aae8a8c64e8fa2f21a3e860ff3:

  - This looks okay but does -Wstring-conversion get us anything but
 syntactic
    contortions like ((answer) != NULL) ?

 End nickm/bug19180_easy code review; I think this is probably okay to
 merge.

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/19180#comment:11>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs