weird seg fault
 

I'm baaaaack.. :)

I have a file, /etc/ips, that is used in IP aliasing. The form of the file is <IP address>:<sbunet mask>. To unbind an IP, I remove its entry in the file and call a script that reloads IPs.

My problem is that I am getting a strange segmentation fault:

Weird part 1: /etc/ips is being removed, and /etc/ips.tmp is getting renamed. After that, all that happens is the function returns; back in main(), all that happens is a break out the switch, and then main() returns! So why a seg fault--won't execution immediately stop?

Weird part 2: I don't understand why I need the line that I have marked /* Why do I need this line? */ Without it, the program does not stop when it reaches EOF in /etc/ips. What I do end up with in /etc/ips is the following:

10.0.0.1:255.255.255.0
10.0.0.3:255.255.255.0
10.0.0.3:(null)

Clearly it is reading past EOF and maskstr is not getting a value, although how ipstr is holding on to its previous value is beyond me. But with that line left out, I don't get the seg fault! What the...??

Thanks for any help or ideas about what could be going on here!

Oh, the entire code is posted here

Re: weird seg fault
 

First: "man strtok" says:
"BUGS
Never use these functions.
"

You need that line because otherwise you would be messing with strings that do not contain anything useful.

You get the segault(s) because you are freeing ipstr and maskstr. strtok() returns char-pointers that point somewhere into the buffer you pass to strok() as the first argument (tempstr in your case). so it's wrong to free() them. Because of this you should not malloc() them either: you lose pointer to the malloc()-ed memory after assigning them the pointer returned by strtok() leaving your malloc()-ed buffers dangling causing a memory leak bug.

Here's an example how to use strok() (if you really want to use strtok(): see its man page, section "BUGS") in your case. I used fgets() here instead of fscanf(), because I personally don't like fscanf(), especially when just reading a single line into a string.

My test file "file.txt" looks like this:
The code:

Hello Heiko,

Brilliant! Works great now. I have some follow-up questions, though:

1. I was under the impression that it was required to allocate space to a string, either by declaring it as an array like char blah[ 80 ], or by declaring it as a pointer to char and using something in the malloc() family to allocate space. Clearly this is not true; it seems like space is automatically allocated to ipstr and maskstr now. Comments on what is going here?

2. I am still failing to see why I had to put in the line

if( feof( in ) || ferror( in ) )
break;

in the loop that reads through the file. I mean, I see how it is preventing the rest of the statements in the loop from writing junk into the file, which is why I put it there. What I don't understand is why we are even at that point. If junk is being read into the string, the while condition should have already been satisfied and the loop should not be executing again:
Let's assume two lines in the file. fscanf reads the first one, the rest of the loop breaks it into tokens, etc. At the bottom of the loop, EOF is checked and we are not there. Execute the loop again, and fscanf should read the second (last) line.

Is EOF not found at this point? Or do we have to attempt to read beyond the last character to have the EOF found?

Thanks again for your excellent help and patience in getting me to understand this!

Whitespace is fscanf()'s fixed seperator/delimiter between the things (tokens) it parses.

So fscanf(in, "%s", tempstr) stops reading when it sees the first whitespace character, such as <tab> <space> <newline> or (apparently) EOF.

You can put as space in fscanf()'s format string to indicate it should "read away" whitespace. In other words, if you put a space after %s ( "fscanf(in, "%s ", tempstr);" ) you can delete the line: "if( feof( in ) || ferror( in ) ) break;"

No. No space for ipstr and maskstr is allocated at all!

They are pointers to the same string in the same memory buffer.
(and you can free() a single buffer only once or you will/may get segfaults.)

The situation is like this after the second call to strtok():
Note that the ':' in the "drawing" is actually not there anymore: it has been replaced with '\0' by the first call to strtok().
strtok() modifies your buffer, replacing a delimiter char with a '\0' character to split the string. This is considered bad behaviour for library functions these days. That's why the the "BUGS" section of its man page says not to use it... You probably know that the '\0' character marks the end of a string. So the buffer contains two strings after the first strtok().

In the first code you posted, you first free() ipstr. This is valid, because it contains the same memory address as tempstr (where the malloc-ed space was assigned to at the beginning). After this first call to free() the other two pointers point to invalid memory, so free()-ing them will/may cause segmentation faults.

I see, they are pointers to space already allocated. That makes good sense now. Guess the seg fault was not so "weird" after all!

Thanks again, so much, for taking the time to explain this to me! (Hopefully to others as well!)

So then, what was happening was that fscanf was "reading away" the EOF, and it wasn't until the next attempt (at the top of loop) to read that it was realized that it was already past the EOF. Makes sense when fscanf's behavior is explained. Once again, thanks tons!

True.
The whitespace seperator of the last line is either a '\n' or a EOF. For scanf to behave consistent (not reading the next whitespace until the format string tells it to), it needs to handle EOF as whitespace, and thus the same way as other whitespace.

Of course, when the last line is not empty (i.e. the last non-empty line does not end in '\n'), scanf() actually hits EOF when it has read the last line. But the end-of-file flag is not set in order to behave consistent.

This is one of the reasons I don't really like scanf(), for doing things like just reading line-by-line. I suppose scanf() can be useful in many situations. But until it proves really handy, I prefer not to use it. This maybe a personal thing though.