Re: snprintf(3) example warns under -Wextra
On Mon, Apr 24, 2017 at 12:01:10AM +0200, Marc Espie wrote: > On Sun, Apr 23, 2017 at 10:16:38PM +0200, Timo Buhrmester wrote: > > > The main difference between you and Theo is that Theo knows what he's > > > talking about. > > > If you want to contribute anything, point out how casting a non-negative > > into to size_t for comparison against another size_t could lead to > > "real errors later whenever the code evolves in any way". Nice straw-man > > with the s/world/code/, too. Sure makes it easier to appear to have a > > point. > You are talking about the code in question changing, I am talking about the environment around it changing. While I agree with you on a rule-of-thumb basis where casts are concerned, I don't see it as much as a blanket rule as you seem to do. Reminds me of people who will religiously try to spread the word that goto has no uses and must be avoided at all costs. > Say, somebody renames variables, and boom, after a few changes, you are > casting a pointer into a size_t. That's an odd thing to happen by renaming only. If the premise is that code can be broken by messing with it, that's trivially true and nothing you need to convince me of. In case you have missed it, the whole "changing the world" thing started as: > Unneccesary casts were among the hardest parts of the jump from 32-bit > to 64-bit, since a cast means "I know better than the compiler". > Well, when the world changes, that cast is suddenly wrong. And all I pointed out that the cast I proposed is safe from such changes in architecture, or new C standards, etc. And I still stand by that claim. Besides, I pointed out a real potential issue with the code that is completely independent of whether or not there is a cast, yet that's being ignored. This is my last reply in this pointless thread.
Re: snprintf(3) example warns under -Wextra
On Sun, Apr 23, 2017 at 10:16:38PM +0200, Timo Buhrmester wrote: > > The main difference between you and Theo is that Theo knows what he's > > talking about. > If you want to contribute anything, point out how casting a non-negative > into to size_t for comparison against another size_t could lead to > "real errors later whenever the code evolves in any way". Nice straw-man > with the s/world/code/, too. Sure makes it easier to appear to have a point. You definitely don't get it, do you ? people routinely change code in sweeping ways, and rely on the compiler to catch a lot of small mistakes. Say, somebody renames variables, and boom, after a few changes, you are casting a pointer into a size_t. Guess what ? you added a cast, so compiler doesn't warn (yes, I just checked, neither gcc nor clang warn on THAT one) I've seen bogus code countless times because a cast hid an horrid mistake. When you review code for mistakes and bugs, every time you see a cast, you have to check the variables, verify something isn't amiss. Because the compiler won't warn you. I have seen this countless times over the years. Like old X code playing mixing pointers and integers, and suddenly no longer working on 64 bit machines. Or somewhat more fun code using that old T *p = (T*)malloc(sz); idiom... Until there's T* on one side, and something else on the other side. Bottom line, each time, I read a cast, I have to slow down, and check it manually. I know the programmer did tell the compiler to explicitly SHUT THE FUCK UP, so the seat belts are GONE, not even detached, but completely BURNT THRU WITH NAPALM. *Whenever* I can avoid a cast, I do.
Re: snprintf(3) example warns under -Wextra
> The main difference between you and Theo is that Theo knows what he's > talking about. If you want to contribute anything, point out how casting a non-negative into to size_t for comparison against another size_t could lead to "real errors later whenever the code evolves in any way". Nice straw-man with the s/world/code/, too. Sure makes it easier to appear to have a point.
Re: snprintf(3) example warns under -Wextra
On Sun, Apr 23, 2017 at 05:12:16PM +0200, Timo Buhrmester wrote: > Except if the world changes... In a way that's still POSIX-compliant. > But why would anyone want to protect themselves from that, right? You're full of it. You're advocating for an unnecessary cast that can actually hide real errors later whenever the code evolves in any way, and now you say that the world can change. The main difference between you and Theo is that Theo knows what he's talking about. Unnecessary casts to silence stupid warnings have always been a plague that leads to large bugs and big security issues.
Re: snprintf(3) example warns under -Wextra
> Timo if you feel the need to be an asshole, please be that asshole > elsewhere. Pot, meet Kettle.
Re: snprintf(3) example warns under -Wextra
> > Otherwise, you can start enabling that option and sending a diff which > > fixes ALL THE WARNINGS IT GIVES IN THE ENTIRE TREE. > I think I'll pass on that. I wasn't aware of how many warnings > a build seems to cause. This must be why NetBSD has -Werror in their > CFLAGS. Timo if you feel the need to be an asshole, please be that asshole elsewhere.
Re: snprintf(3) example warns under -Wextra
> > Otherwise people might fall into a habit > > of ignoring warnings [that may point to actual problems]. > > People might fall into the habit of ignoring a warning from an > extension to C provided by a single compiler? > > I doubt it. Doubt away. I find it more than obvious that telling people to ignore some warnings will make them more likely to also ignore and underestimate other warnings. Provided they even notice the serious warnings in a stream of harmless warnings. Whatever. > There is no point in silencing the warning, since the warning is > from an extension to C which is bullshit. What extension of C are you talking about? > > (*) looking at POSIX, snprintf is not required to return -1, > > but only "a negative value". So it's not truly safe either way. > > It cannot occur in this code in any case. Except if the world changes... In a way that's still POSIX-compliant. But why would anyone want to protect themselves from that, right? > You are barking up the wrong tree. Obviously I am. > Otherwise, you can start enabling that option and sending a diff which > fixes ALL THE WARNINGS IT GIVES IN THE ENTIRE TREE. I think I'll pass on that. I wasn't aware of how many warnings a build seems to cause. This must be why NetBSD has -Werror in their CFLAGS.
Re: snprintf(3) example warns under -Wextra
> > The code is already safe. > It is reasonably safe(*) and triggers a warning. That's a good reason > to silence the warning. No. The warning is a false extension to C. In C, int and sizeof can be compared safely in this circumstance. > Otherwise people might fall into a habit > of ignoring warnings [that may point to actual problems]. People might fall into the habit of ignoring a warning from an extension to C provided by a single compiler? I doubt it. > I just pointed out a safe way to silence the warning, without it > potentially blowing up in a changed world. There is no point in silencing the warning, since the warning is from an extension to C which is bullshit. > (*) looking at POSIX, snprintf is not required to return -1, > but only "a negative value". So it's not truly safe either way. It cannot occur in this code in any case. You are barking up the wrong tree. Otherwise, you can start enabling that option and sending a diff which fixes ALL THE WARNINGS IT GIVES IN THE ENTIRE TREE. Which we will gladly delete also. Because the tree is written in C, and C allows that idiom, because it is safe.
Re: snprintf(3) example warns under -Wextra
> The code is already safe. It is reasonably safe(*) and triggers a warning. That's a good reason to silence the warning. Otherwise people might fall into a habit of ignoring warnings [that may point to actual problems]. I just pointed out a safe way to silence the warning, without it potentially blowing up in a changed world. (*) looking at POSIX, snprintf is not required to return -1, but only "a negative value". So it's not truly safe either way.
Re: snprintf(3) example warns under -Wextra
> > Well, when the world changes, that cast is suddenly wrong. > > I.e. instead of > > if (ret == -1 || ret >= sizeof(buffer)) > one could use > > if (ret < 0 || (size_t)ret >= sizeof(buffer)) > > And be safe from a changing world. The code is already safe.
Re: snprintf(3) example warns under -Wextra
> Well, when the world changes, that cast is suddenly wrong. I.e. instead of > if (ret == -1 || ret >= sizeof(buffer)) one could use > if (ret < 0 || (size_t)ret >= sizeof(buffer)) And be safe from a changing world.
Re: snprintf(3) example warns under -Wextra
-Wextra is stupid.
It is trying to persuade patterns that ignore the rules of standard C.
In particular for this situation:
Never add extra costs. Adding casts everywhere is HIGHLY ERROR PRONE.
Unneccesary casts were among the hardest parts of the jump from 32-bit
to 64-bit, since a cast means "I know better than the compiler".
Well, when the world changes, that cast is suddenly wrong.
The people adding these things to the compiler are trying to break
the language, leading the user community astray.
> The example proper usage of snprintf(3) (under Caveats) evokes a warning
> when compiled with -Wextra. I presume casting ret to unsigned int would
> be safe, but I'll defer to those who know the nuances.
>
>
> #include
>
> int
> foo(char* string) {
> char buffer[128];
> int ret = snprintf(buffer, sizeof(buffer), "%s", string);
> if (ret == -1 || ret >= sizeof(buffer))
> return 1;
> return 0;
> }
>
> % cc -c -Wextra foo.c
> foo.c: In function 'foo':
> foo.c:7: warning: comparison between signed and unsigned
>
snprintf(3) example warns under -Wextra
The example proper usage of snprintf(3) (under Caveats) evokes a warning
when compiled with -Wextra. I presume casting ret to unsigned int would
be safe, but I'll defer to those who know the nuances.
#include
int
foo(char* string) {
char buffer[128];
int ret = snprintf(buffer, sizeof(buffer), "%s", string);
if (ret == -1 || ret >= sizeof(buffer))
return 1;
return 0;
}
% cc -c -Wextra foo.c
foo.c: In function 'foo':
foo.c:7: warning: comparison between signed and unsigned
