Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Cleaning up X server warnings

keithp.com

Post reply on HN
Print view View original

11–20 of 34 posts

Re: Cleaning up X server warnings

#11

Post by viraptor »

unwind wrote: post #9
raverbashing wrote: post #2

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 s…

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-visi…

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?

Re: Cleaning up X server warnings

#12

Post by yxhuvud »

IgorPartola wrote: post #8

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…

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.

Re: Cleaning up X server warnings

#13

Post by joosters »

viraptor wrote: post #11
unwind wrote: post #9

Earlier quoted context omitted.

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-visi…

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?

Also, if you're passing data between two processes (e.g. your code fork()d a child), you can be sure that the data format is going to match, no packing required. (still need to ensure alignment in memory though)

Re: Cleaning up X server warnings

#14

Post by joosters »

yxhuvud wrote: post #12
IgorPartola wrote: post #8

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…

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.

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?

Re: Cleaning up X server warnings

#15

Post by mikeash »

viraptor wrote: post #11
unwind wrote: post #9

Earlier quoted context omitted.

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-visi…

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?

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.

Re: Cleaning up X server warnings

#16

Post by sophacles »

raverbashing wrote: post #2

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 s…

The notion of warnings is not that it the behavior is broken, or even wrong, but rather dangerous. A good solution would be to include a pragma or such that that would allow programmers to say "I recognize this is dangerous, but it is correct".

Even better would be a pragma that allows you to say the above, and also point to a test/test_suit that verifies that the specific case is still working right and warns on "something has changed, test foo no longer holds, here's the associated warning as a hint".

Re: Cleaning up X server warnings

#17

Post by raverbashing »

sophacles wrote: post #16
raverbashing wrote: post #2

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 s…

The notion of warnings is not that it the behavior is broken, or even wrong, but rather dangerous. A good solution would be to include a pragma or such that that would allow programmers to say "I recognize this is dangerous, but it is correct". Even better would be a pragma that allows you to say the above, and also point to a test/test_suit that verifies that the specific case is still working right and warns on "so…

Well, some warnings are valid, but yeah, the "I recognize this is dangerous, but it is correct" makes perfect sense.

Re: Cleaning up X server warnings

#18

Post by weland »

viraptor wrote: post #11
unwind wrote: post #9

Earlier quoted context omitted.

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-visi…

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?

> 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?

You know that at this moment they all support the right __attribute__((packed)) et co.. Bonus points for endianness crap.

Re: Cleaning up X server warnings

#19

Post by mtklein »

joosters wrote: post #6
raverbashing wrote: post #2

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 s…

I thought that the issue is the const-ness of the variables, and nothing to do with casting void* ->sometype* Essentially, they are making use of const pointers to ensure that the code doesn't change the data. (gcc would throw a warning if you did). BUT: the problem comes when you want to free() the data. A strict interpretation of C would be that you can't free() something that's const, because it clearly is alterin…

On a related note, if you're using new and delete you can simply delete a pointer-to-const without jumping through any const_cast hoops, though I often wish I had a way of blocking that for APIs where the callee doesn't take ownership of a const Foo*.

Re: Cleaning up X server warnings

#20

Post by raverbashing »

mikeash wrote: post #15
viraptor wrote: post #11

Earlier quoted context omitted.

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?

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 cod…

"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

And yes, GIMP reads field by field https://git.gnome.org/browse/gimp/tree/plug-ins/file-bmp/bmp...

Post reply on HN