Thread: use of printf prevents segfault!

"So ray[] is a singe element array, and using that for nofl > 0 is invalid. You need to fill in number of elements in the brackets to make sure you have enough space. If you can't determine the number you need, it may be better to use some sort of dynamic allocation (malloc and friends)." -matsp

"You can't do that. Arrays can't grow dynamically. It's pretty much the definition of an array that it has a set size. Once an array has a certain set of dimensions it can't be changed.
Btw, there are no harmless warnings." -- QuantumPete

Hmmm...this makes things rather interesting, and explains why I didn't know you could do what I did because evidently I am not suppose to. Which is a shame because

Code:

struct thestruct {
[...]
} ray[];
int main () {
     short int nofl, i;
     [...]
     for (i=0; i<nofl; i++) {
          ray[i].member = whatever;
     }

works so very, very well (except for the warning) it's almost perl. I am sure it's true that "there are no harmless warnings" but, while I am new to C (obviously) I've been compiling stuff from source in linux for years and years, and I have never ever seen a program of significant size, including the kernel, gcc and glibc itself, etc. that did not produce hundreds or thousands of such warnings while compiling and which evidently the people who designed them comnpletely expected, because it's de facto to find some comment in the README (or whereever) like this "Don't worry if you see a number of warnings produced by the compiler, these are normal." Also, I've already discovered that in my own standard issue, out of the Fedora 7 box libc 2.6, using "strcasestr" will produce a warning no matter what (just try it). Etc. So either I believe strcasestr is an illegal command, period, or assume that some warnings are less significant and appropriate than others.

Anyway, thanks for the warnings. b/t/w QuantumPete misunderstands -- this was the fix, not the problem: "the warning tells you that you created your array with a size of 1. Then when you go to using it, you're most likely writing/reading past the end of the array, causing your segfault". I haven't had a segfault since the change, just the warning, the array grows fine, and so on.

But if anyone has an example of how to grow the array using malloc and realloc "properly" please post it (how do you cast an array of structs -- (struct **)realloc(?bytes?!!) -- i haven't tried...).

In the meantime I heartily recommend people try ray[] for themselves...

nofl so far is less than 100.

ps. if this turns out to be a problem (it's going into a midsize GUI app which will get much play-testing) I promise I'll come back and admit it...

Try adding:

Code:

ray[];
int array[10];
...
// inside main:
{
for(i = 0; i < 10; i++)
{
   array[i] = i * 3;
};
... access ray[]
...
for(i = 0; i < 10; i++)
{
   if (array[i] != i * 3)
       printf("array corrupted at %d, got %d, expected %d\n", i, array[i], i * 3);
}

I bet you that you will get "array corrupted".

Arrays do not grow automatically in C or C++ - they may seem to, but only because you don't realize you've overwritten something else.

--
Mats

Originally Posted by MK27

In the meantime I heartily recommend people try ray[] for themselves...

nofl so far is less than 100.

hmm... I suggest that you post a minimal example of what is actually in the [...].

I see two possibilities: you are accessing your array out of bounds but escaping unscathed by luck, or the C99 variable length array language feature is at work. If it is the latter, fine, but if it is the former, then you may pay a heavy price for your mistake in the future.

Originally Posted by laserlight

hmm... I suggest that you post a minimal example of what is actually in the [...].

I see two possibilities: you are accessing your array out of bounds but escaping unscathed by luck, or the C99 variable length array language feature is at work. If it is the latter, fine, but if it is the former, then you may pay a heavy price for your mistake in the future.

Since it is a global variable, I expect that it won't be "variable length array". And the warning the compiler gives is explcitly saying that it's giving a size of 1 - so there is only one element in the array called ray.

--
Mats

Originally Posted by cpjust

Yes, I've noticed this too, and it's a damned shame...
It's true that some warnings can be safely ignored in some instances (sometimes the compiler just over-reacts when it sees something potentially dangerous), but other times that same warning can indicate a real problem.
The problem with ignoring warning messages without fixing them is that eventually your project spits out hundreds (or thousands) of warnings, and if you make a change in the code and a new warning pops up in the middle of all that mess that is in fact a real problem, chances are you won't even notice it. So to that end, it's better to fix all warnings right from the start, or at least use #pragma's to silence specific warnings (in the smallest scope possible).

Completely agree - projects should have no warnings [1]. And this is why certain projects use the rule that you have to use -Werror when compiling - that way, all warnings become errors, and you have to fix them. Nearly all warnings can be aovided one way or another - you may have to add some extra bits (a cast, an extra set of parenthesis, an initial value for a variable, etc) to make the warning go away, but generally it's possible and most often not enough extra work.


[1] Of course, there are situations where you are damned to eternity. I remember having a switch/case set where I had a default to cover "random acts of madness" in the compiler - gcc then comlained about "unreachable code". I removed it, and now it complained that "not all cases of the enum was covered" - how do you want it?

--
Mats

I'm not sure what matsp's example is suppose to demonstrate -- that "accessing ray[]" will corrupt the other array? Why? Because the compiler handles arrays in the same way and hence the best chance of an overwrite is with another array?

You & laserlight could be right about this, I'll let you know when my tish fits the hand. I'm not sure if ray[] will count in C99 "variable length array":
http://gcc.gnu.org/onlinedocs/gcc/Variable-Length.html
But at least there's the possibility...boldly going and all that...

[...] includes the determination of nofl (to recap: I'm doing what must be a common task -- reading a text file of indeterminate length, each line of which (eg. "name number section") is translated into a struct. I guess I could set ray[10000] or something.

But as of yet, there's no problem...and this is in a gtk_main() context, so there's a lot of storage going on...

I may have to investigate this "pragma" thing.

nofl IS NOT global, and if you think about reading lines not, perhaps, even necessary. The warning about size of one is probably because ray[] appears to have no size at all. But in the for (i) loop, ray[i] does function and retain it's value globally.

ps. my point about the warnings intrinsic to compiling, eg. the linux kernel and C libraries, would be that even the people who built all this can't seem to get rid of them. The one with strcasestr is:

warning: assignment makes pointer from integer without a cast

which doesn't appear with strstr (tho their use is identical).

Regarding what you example is supposed to show: I choose an array because that the simplest way to create a few variables worth of memory that I could write some "pattern" to. Did you actually try to implement that?

Your code probably will crash when you reach the next 4K boundary of ray - since we don't know what ray contains, it's impossible to predict when that would happen (and it may well be that other data is stored AFTER ray, and that it just happens that overwriting that causes nothing bad that you can detect). This is the problem with undefined behaviour - it's like crossing the road when "don't walk"/"red man" showing - some days you'd get across the road fine, other days you'd get run over by a bus, and further other days, Police Constable Plod grabs you by the collar and tells you off [yeah, ok, that's unlikely - but still...].

As to strcasestr(), are you sure you added the red part in your source code?

Code:

#define _GNU_SOURCE
#include <string.h>

--
Mats

nofl IS NOT global, and if you think about reading lines not, perhaps, even necessary. The warning about size of one is probably because ray[] appears to have no size at all. But in the for (i) loop, ray[i] does function and retain it's value globally.

Based on the available information (I still cannot fully understand variable length arrays as specified by C99 just by reading C99), I think matsp is right. ray is a global, so variable length arrays do not come into play here (from what I understand, the length of a variable length array is fixed for the duration of its lifetime: the length is variable in the sense that it is not fixed at compile time). Therefore it is just an array of length 1, in which case you have a case of array out of bounds access, which is a Bad Thing.

Okay! I tried it and matsp WAS WRONG!

Code:

#include <stdio.h>

struct thest {
        int one;
        int two;
} ray[];
int array[10];

int main () {   
        int i;
        for (i=0; i<10; i++) {
                array[i] = i*3;
        }   
        for (i=0; i<=4; i++) {
                ray[i].one = i;
                ray[i].two = i*2;
        }   
        for (i=0; i<=4; i++) printf("ray[&#37;d]: %d\t%d\n", i,ray[i].one,ray[i].two);
        for (i=0; i<10; i++) {
                if (array[i] != i*3) printf("array[%d] isn't %d, it's %d...\n",i,i*3,array[i]);
                else puts("No problem");
        }
}

Sorry the "code" tags don't work here (?!?). But the output is:

Code:

ray[0]: 0       0
ray[1]: 1       2
ray[2]: 2       4
ray[3]: 3       6
ray[4]: 4       8
No problem
No problem
No problem
No problem
No problem
No problem
No problem
No problem
No problem
No problem

But I clearly haven't hit a 4k boundary yet. Also,
#define _GNU_SOURCE
didn't make any difference with strcasestr.

Of course, now all subsequent unexplained segfaults will leave me paranoid...

A Very Bad Thing. Probably the only thing that is saving you is the fact that "ray" is a global. Thus it will be stored in the global part of memory, which on many architectures is at the bottom of the memory and will therefore likely grow into your heap. If you don't have much heap allocated memory, you might never see a problem, but it's definitly not safe and the slightest code change could break your program.
To which, I might add, that this kind of buffer overflow can be used as an exploit to gain control over your software and run arbitrary code. So it's not just unsafe but insecure as well.
On warnings: You should try to eliminate all warnings (and not by supressing them), the only time that you can ignore a warning is if you know why it's occuring, it's not trivial to fix and you understand the consequences of allowing the warning.

QuantumPete

Originally Posted by MK27

Okay! I tried it and matsp WAS WRONG!

Erm, actually, no he's not. It's up to the compiler how to arrange your variables in memory and it could well have put array before ray. Maybe because it arranges them by descending size or because of alignment. Pick a different compiler and you may well get a different result.

QuantumPete

ARRRGHHHH!

Looks like i will be boldly returning in shame. I'm up over 200 lines in my little app and ray[] has created insurmountable problems. Oh well.

Thanks for all your input and tolerating my bull-headed ignorance.

What compiler are you using, and on what processor architecture, since with gcc-mingw shows the problem, and MS Visual Studio .Net refuses to build an executable (but if I change the [] to 1, it compiles correctly and produces the error messages I expect).

But as QuantumPete says, the compiler isn't obliged to put the variables in the order that you have them in the source - I've never seen a compiler do that, but it's not impossible.

--
Mats