Wireshark-dev: Re: [Wireshark-dev] #ifdef mess

From: João Valverde <joao.valverde@xxxxxxxxxxxxxxxxxx>
Date: Wed, 30 Mar 2016 02:51:40 +0100


On 29-03-2016 23:48, Joerg Mayer wrote:
On Tue, Mar 29, 2016 at 03:34:38PM +0100, João Valverde wrote:
On 28-03-2016 23:30, Joerg Mayer wrote:
I've been meaning to write this mail for some years now but finally got around to it.

Earlier today I committed 30900b443b85a7e760d703ca3d6efe61df4fe623, which I'm
incredibly unproud of because of readablity:

  static void
-get_reordercap_runtime_info(GString *str _U_)
+get_reordercap_runtime_info(
+#if defined(HAVE_LIBZ) && !defined(_WIN32)
+    GString *str)
+#else
+   GString *str _U_)
+#endif
  {

It fixes the error at hand, but that is about all the good I can say about it.

It's only an error because -Werror=used-but-marked-unused was
enabled. Since the semantics of _U_ are *possibly* unused variable,
neither the warning nor the #ifdef should exist IMO.

I don't follow your logic here. Couldn't we by the same logic just remove the _U_
and then blame the resulting warning turning erro on the unused warning? The idea
or turning on many warnings is to find coding/logic bugs. Blaming the problem on
turning on the warning is like shooting the messenger, i.e. plain wrong.

I'm just trying to say that the problem with the code, if there is one, is in the

  #if defined(HAVE_LIBZ) && !defined(_WIN32)

body condition, not the unused variable attribute. That is a proper use of _U_ because the argument may or may not be used, so the attribute is rightfully telling the compiler not to emit warnings about it.

In this particular case I think the used-but-marked-unused warning should be ignored/dismissed, not fixed with even more #ifdefs.

I fully agree we have too many #ifdefs in general.