
Cleaning up X server warnings - ruda
http://keithp.com/blogs/xserver-warnings/
======
raverbashing
Interesting.

Interesting how the -Wcast-qual warnings are _unfixable_ , so it shouldn't
exist. Damned if you do, damned if you don't. A lot of warnings seem to fall
on this.

Or warnings that would make sense on stricter languages but doesn't make sense
on C _exactly because of what you can do with C_ , like, for example, reading
serialized data, then casting it to (MyStructure *)

And a little bit offtopic, but GTk have some unfixable warnings as well,
something like "blah is not implemented in this platform" so, if I have to use
this, but BLAH is not implemented, thanks for warning me, but shut up if I
can't fix it

~~~
unwind
Reading serialized data and then casting it into a structure type is ill-
defined, very bad practice, and there's nothing in C that tells you it's a
good or safe thing to be doing. So, please don't do that.

The exact layout of a structure in memory is up to the compiler, and can
easily change with compiler options, even if the compiler itself and the
target architecture and platform remain the same.

An externally-visible serialization format should of course be stable, and not
depend on the compiler used to build the code, or the hardware platform which
might have ideas of how various fields should be aligned (or even how to order
the bytes in integers larger than 8 bits).

Serializing data is something you do, just blindly copying the run-time
representation that you have to an external medium is _not_ serialization. You
should do it field by field, with a well-defined format chosen for each field.

~~~
viraptor
That's the theory, sure. But in practice, if you know all your targets support
the right "#pragma pack" and "__attribute__((packed))" and other markers that
you use - what's wrong with making use of them?

~~~
mikeash
The fact that you can't know all of your _future_ targets will support that is
a problem.

Endian issues are also a big problem here. The need to byte-swap everything
eliminates a lot of the convenience.

It's also way too easy to make breaking changes to the struct, since there's
no standard way to mark a struct as being something that you
serialize/deserialize, and normally you can change struct fields at will in C
code.

Just take the extra five minutes to write code to translate between your
struct and a stream of bytes. It'll be easier to understand, less risky, and
more compatible.

~~~
raverbashing
"The fact that you can't know all of your future targets will support that is
a problem."

I don't see this as much of a problem as endianness, but yeah, maybe, x86 made
us accustomed. And you can use portable types, defined on headers (linux does
that, like u8, u16, etc)

But sometimes you know your target won't change (for a long time)

"It's also way too easy to make breaking changes to the struct, since there's
no standard way to mark a struct as being something that you
serialize/deserialize, and normally you can change struct fields at will in C
code."

In the same way you can break your software doing any change. You can mark it
with a naming convention but the easiest way is seeing the "pack" directives
on it. And unit tests

"Just take the extra five minutes to write code to translate between your
struct and a stream of bytes. It'll be easier to understand, less risky, and
more compatible. "

Well, it's not five minutes. And it makes your program slower (for the most
convoluted data types). A simple example:

[http://en.wikipedia.org/wiki/BMP_file_format](http://en.wikipedia.org/wiki/BMP_file_format)

And yes, GIMP reads field by field
[https://git.gnome.org/browse/gimp/tree/plug-ins/file-
bmp/bmp...](https://git.gnome.org/browse/gimp/tree/plug-ins/file-bmp/bmp-
read.c?h=gimp-2-6)

~~~
mikeash
Does it really make your program slower? That padding you have to eliminate
for this technique is added for speed, after all. The initial read may be
faster, but every access to the data is going to be slower.

I'm not quite sure what the BMP file format is supposed to be an example of,
especially since file formats are separate from techniques used to read or
write them.

------
IgorPartola
I always wondered why so many FOSS projects always generate warnings. I always
assumed that the large mature projects like the X server did this because they
were performing some crazy optimization that the compiler was too naive to
know about. I never had to do this myself, so I thought that the pro's used
their arcane knowledge to eek out an extra 0.1% performance out of something
for the benefit of everyone. Turns out that the truth is so much less
interesting.

~~~
yxhuvud
Often, it will be because the projects are older than the warnings. And when
the amount of warnings cross a threshold, they will not be fixed.

~~~
joosters
Yes, definitely this. If there are hundreds of warnings, there's no motivation
to fix any of them. If there are none, hopefully no-one wants to add the code
that creates one.

Perhaps they now need to compile the project with warnings-as-errors to
enforce clean code?

------
tenfingers
It always surprised me that GCC has no source-level way of controlling
compiler warnings.

Many programmers (and gcc apparently) live with the assumption that compiler
warnings _must_ _always_ be fixed. This is largely false. Compiler warnings
are there to help with additional information, not to be something to fix.
There are many cases where the compiler issues a warning which must be
ignored.

Many compilers support pragmas to silence a _specific_ warning class in a
specific point in the code. IMHO it's a much better approach to silence just a
single warning instance where you know the compiler is getting the wrong
assumption than silencing a whole warning class like the article suggests.

But it seems that in GCC you have no choice. Instead, code written with/for
gcc often silences the warning by inserting some dummy code. For instance, the
"unitialized warning" is usually fixed by assigning the variable to itself.
It's shitty to look at, and someone that doesn't know this trick might wonder
why this is being done.

I've reported a feature request many years ago, and it was quickly closed...

~~~
mitchty
I thought thats exactly what these are for:
[http://gcc.gnu.org/onlinedocs/gcc/Diagnostic-
Pragmas.html](http://gcc.gnu.org/onlinedocs/gcc/Diagnostic-Pragmas.html)

Also work for clang like so (replace clang with GCC for its version):

    
    
        #pragma clang diagnostic push
        #pragma clang diagnostic ignored "-Wsomeignoredwarning"
        triggered by this code
        #pragma clang diagnostic pop

