Programming Forums > Application Development > C

nnxion · Nov 26th, 2005, 8:21 AM

I'm trying to read a line from a file and then return the line. I want it to be dynamically allocated so no matter how long the line is, I will always get it, unless memory runs out. I know the problem is occuring at realloc, I'm not seeing what is going wrong. This is my readLine function:

(Toggle Plain Text)

char *readLine(FILE *iop)
{
	int c, i = 0;
	char *cs;
	char *s  = "";

	cs = s;
	while ((c = getc(iop)) != EOF)
	{
		i++;
		s = (char*) realloc (s, i * sizeof(char));
		if (s == NULL)
		{
			puts ("Error (re)allocating memory");
			exit (1);
		}
		if ((*cs++ = c) == '\n')
			break;
	}
	*cs = '\0';
	return (c == EOF && cs == s) ? NULL : s;
}

Is there a better and more elegant way?

In my main where I am calling readLine, can I use:

(Toggle Plain Text)

char *line = NULL;
..... // lots of other lines
while((line = readLine(rfs)) != NULL)
	{
		printf("the line says: %s\n", line);
	}

Or do I have to allocate memory in main too?

DaWei · Nov 26th, 2005, 8:32 AM

You don't say what the problem is, just that one is occurring. You don't need to allocate in main, but you need to eventually free there. You say you want to read a line and return it, but you don't say what your larger goal is. There could be a better approach, if there IS a larger goal, a file-sized one, say.

nnxion · Nov 26th, 2005, 8:45 AM

Yeah, I had considered that, but I once made a file of 10 GB so if I opened that it shouldn't crash

. Well maybe it should, but anyway, I want to make a beautifier, that should insert some newlines, and spaces, or delete some. So basically I would like a buffer 2 times that of the current line. So I have enough of a working area.

The pseudocode in my mind was something like:
-open the file(s)
-read a line while there are more lines (not end of file)
-make a buffer twice as large as the read line
-parse the line
-write the line (to the same or another file)
-close the file(s)

I'm not sure what the problem is here, it just crashes, I know it happens at realloc (with the debugger) though.

DaWei · Nov 26th, 2005, 9:23 AM

This is just an off-the-top-of-my-head assessment. Personally, I'd probably just declare a large buffer on the presumption that a beautifiable file wouldn't have lines over, say, 2K or so. If I wanted to use the heap, though, I would malloc some suitable buffer in the surrounding procedure (main, say), and not the line fetching procedure. This is so I malloc'ed and freed from the same locality. I'd pass a pointer to the buffer, as well as its current length. Inside the procedure, I'd use fgets, knowing the length of the buffer. The absence of a newline at the end of the buffer would indicate that the buffer was too short. I would return status indicating that failure. Main (or whatever) would then do a realloc and try again. That precludes having to do an alloc on every call (or making the pointer static).

grumpy · Nov 26th, 2005, 4:33 PM

You are actually asking for trouble in your readLine() function in two ways that yield undefined behaviour.

realloc() assumes the pointer it is given is allocated with malloc, calloc, or realloc. Undefined behaviour therefore results from;

(Toggle Plain Text)

    char *s = "";    /*  not created with one of those functions */
    s = realloc(s, i);

Your variable cs is becoming a dangling pointer if realloc() actually changes the string.

(Toggle Plain Text)

   char *cs = s;
   s = realloc(s, i);
   if ((*cs++ = c) == '\n')    // undefined behaviour

There are a few other minor things. In C, it is unnecessary and considered bad style to cast the result of malloc/realloc/calloc (although it is necessary in C++). sizeof(char) is also defined to be one. A line that only does "i++;" is more accurately expressed as "++i;" (it makes no difference to most compilers, which will optimise the difference away, but i++ involves creating a temporary).

The fact that (if realloc() actually resizes) changing s in your loop means that the test "cs == s" will always yield false and your function will return NULL.

nnxion · Nov 26th, 2005, 7:06 PM

Thanks Robert for pointing that out, I love learning things, you sure taught me here =). I thought it was like that with the i++ and the i++, but David got me confused here. But I think David meant that creating an integer means almost nothing nowadays. And David; your 'off-the-top-of-my-head assessments' are excellent, a nice and elegant solution.

DaWei · Nov 26th, 2005, 7:15 PM

I'll stand by my original statement. The following is UNOPTIMIZED code from my compiler, asm with the C++ statement appearing as a comment.

(Toggle Plain Text)

//   int i = 0;
00411445  mov         dword ptr [i],0 
//    i++;
0041144C  mov         eax,dword ptr [i] 
0041144F  add         eax,1 
00411452  mov         dword ptr [i],eax 
//    ++i;
00411455  mov         eax,dword ptr [i] 
00411458  add         eax,1 
0041145B  mov         dword ptr [i],eax

nnxion · Nov 26th, 2005, 7:23 PM

LOL, I'm totally clueless, so I will just follow anybody that sounds credible here

I really dont care about it anyway.

nnxion · Nov 26th, 2005, 10:17 PM

One thing I haven't resolved yet is that when using fgets, and the line cannot be put into buffer, then I increase the buffer size, but I want to re-get the line, but fgets has already gotten it out of the stream. How would I read the line from the beginning again? Is there a unfgets like an ungetc? :p

grumpy · Nov 27th, 2005, 12:38 AM

OK;

1) Dawei is correct in pointing out that the difference between "i++;" and "++i;" is often insignificant in practice --- and he gives an example, in which there is no difference with his compiler. I would expect most modern compilers would do a similar thing, which makes the difference a moot point. I mentioned it in my earlier post mostly in the interests of completeness (and believing code should be structured around what you intend with no potential for unintended side-effects).

2) There is no unfgets() or conceptual equivalent. I personally would not bother with trying to restart reading the line. With fgets(), a practical clue that the line being read is longer than the buffer would be a lack of a '\n' in the string (as a sanity check in debugging, the buffer probably would be also completely filled eg strlen(buffer) == BUFFER_SIZE - 1, but there is no practical need to rely on that in working code). Then all you need to do is call fgets() again to get more of the line ......

Nov 26th, 2005, 8:21 AM	#1
nnxion Programming Guru Join Date: Jun 2005 Location: elemental plane Posts: 1,429 Rep Power: 5	realloc not functioning as expected I'm trying to read a line from a file and then return the line. I want it to be dynamically allocated so no matter how long the line is, I will always get it, unless memory runs out. I know the problem is occuring at realloc, I'm not seeing what is going wrong. This is my readLine function: (Toggle Plain Text) char readLine(FILE iop) { int c, i = 0; char cs; char s = ""; cs = s; while ((c = getc(iop)) != EOF) { i++; s = (char) realloc (s, i sizeof(char)); if (s == NULL) { puts ("Error (re)allocating memory"); exit (1); } if ((cs++ = c) == '\n') break; } cs = '\0'; return (c == EOF && cs == s) ? NULL : s; } char readLine(FILE iop) { int c, i = 0; char cs; char s = ""; cs = s; while ((c = getc(iop)) != EOF) { i++; s = (char) realloc (s, i sizeof(char)); if (s == NULL) { puts ("Error (re)allocating memory"); exit (1); } if ((cs++ = c) == '\n') break; } cs = '\0'; return (c == EOF && cs == s) ? NULL : s; } Is there a better and more elegant way? In my main where I am calling readLine, can I use: (Toggle Plain Text) char line = NULL; ..... // lots of other lines while((line = readLine(rfs)) != NULL) { printf("the line says: %s\n", line); } char line = NULL; ..... // lots of other lines while((line = readLine(rfs)) != NULL) { printf("the line says: %s\n", line); } Or do I have to allocate memory in main too? __________________ "Employ your time in improving yourself by other men's writings, so that you shall gain easily what others have labored hard for." -- Socrates

Nov 26th, 2005, 8:32 AM	#2
DaWei Resident Grouch Join Date: Jun 2005 Posts: 6,453 Rep Power: 10	You don't say what the problem is, just that one is occurring. You don't need to allocate in main, but you need to eventually free there. You say you want to read a line and return it, but you don't say what your larger goal is. There could be a better approach, if there IS a larger goal, a file-sized one, say. __________________ Abstraction doesn't make it impossible to write bad code; it makes it possible to write superior code. Contributor's Corner: Grumpy on C++ Exceptions DaWei on Pointers

Nov 26th, 2005, 8:45 AM	#3
nnxion Programming Guru Join Date: Jun 2005 Location: elemental plane Posts: 1,429 Rep Power: 5	Yeah, I had considered that, but I once made a file of 10 GB so if I opened that it shouldn't crash . Well maybe it should, but anyway, I want to make a beautifier, that should insert some newlines, and spaces, or delete some. So basically I would like a buffer 2 times that of the current line. So I have enough of a working area. The pseudocode in my mind was something like: -open the file(s) -read a line while there are more lines (not end of file) -make a buffer twice as large as the read line -parse the line -write the line (to the same or another file) -close the file(s) I'm not sure what the problem is here, it just crashes, I know it happens at realloc (with the debugger) though. __________________ "Employ your time in improving yourself by other men's writings, so that you shall gain easily what others have labored hard for." -- Socrates

Nov 26th, 2005, 9:23 AM	#4
DaWei Resident Grouch Join Date: Jun 2005 Posts: 6,453 Rep Power: 10	This is just an off-the-top-of-my-head assessment. Personally, I'd probably just declare a large buffer on the presumption that a beautifiable file wouldn't have lines over, say, 2K or so. If I wanted to use the heap, though, I would malloc some suitable buffer in the surrounding procedure (main, say), and not the line fetching procedure. This is so I malloc'ed and freed from the same locality. I'd pass a pointer to the buffer, as well as its current length. Inside the procedure, I'd use fgets, knowing the length of the buffer. The absence of a newline at the end of the buffer would indicate that the buffer was too short. I would return status indicating that failure. Main (or whatever) would then do a realloc and try again. That precludes having to do an alloc on every call (or making the pointer static). __________________ Abstraction doesn't make it impossible to write bad code; it makes it possible to write superior code. Contributor's Corner: Grumpy on C++ Exceptions DaWei on Pointers

Nov 26th, 2005, 4:33 PM	#5
grumpy Programming Guru Join Date: Jun 2005 Location: Adelaide, South Australia Posts: 1,221 Rep Power: 5	You are actually asking for trouble in your readLine() function in two ways that yield undefined behaviour. realloc() assumes the pointer it is given is allocated with malloc, calloc, or realloc. Undefined behaviour therefore results from; (Toggle Plain Text) char s = ""; / not created with one of those functions / s = realloc(s, i); char s = ""; /* not created with one of those functions / s = realloc(s, i); Your variable cs is becoming a dangling pointer if realloc() actually changes the string. (Toggle Plain Text) char cs = s; s = realloc(s, i); if ((cs++ = c) == '\n') // undefined behaviour char cs = s; s = realloc(s, i); if ((*cs++ = c) == '\n') // undefined behaviour There are a few other minor things. In C, it is unnecessary and considered bad style to cast the result of malloc/realloc/calloc (although it is necessary in C++). sizeof(char) is also defined to be one. A line that only does "i++;" is more accurately expressed as "++i;" (it makes no difference to most compilers, which will optimise the difference away, but i++ involves creating a temporary). The fact that (if realloc() actually resizes) changing s in your loop means that the test "cs == s" will always yield false and your function will return NULL.

Nov 26th, 2005, 7:06 PM	#6
nnxion Programming Guru Join Date: Jun 2005 Location: elemental plane Posts: 1,429 Rep Power: 5	Thanks Robert for pointing that out, I love learning things, you sure taught me here =). I thought it was like that with the i++ and the i++, but David got me confused here. But I think David meant that creating an integer means almost nothing nowadays. And David; your 'off-the-top-of-my-head assessments' are excellent, a nice and elegant solution. __________________ "Employ your time in improving yourself by other men's writings, so that you shall gain easily what others have labored hard for." -- Socrates

Nov 26th, 2005, 7:15 PM	#7
DaWei Resident Grouch Join Date: Jun 2005 Posts: 6,453 Rep Power: 10	I'll stand by my original statement. The following is UNOPTIMIZED code from my compiler, asm with the C++ statement appearing as a comment. (Toggle Plain Text) // int i = 0; 00411445 mov dword ptr [i],0 // i++; 0041144C mov eax,dword ptr [i] 0041144F add eax,1 00411452 mov dword ptr [i],eax // ++i; 00411455 mov eax,dword ptr [i] 00411458 add eax,1 0041145B mov dword ptr [i],eax // int i = 0; 00411445 mov dword ptr [i],0 // i++; 0041144C mov eax,dword ptr [i] 0041144F add eax,1 00411452 mov dword ptr [i],eax // ++i; 00411455 mov eax,dword ptr [i] 00411458 add eax,1 0041145B mov dword ptr [i],eax __________________ Abstraction doesn't make it impossible to write bad code; it makes it possible to write superior code. Contributor's Corner: Grumpy on C++ Exceptions DaWei on Pointers

Nov 26th, 2005, 7:23 PM	#8
nnxion Programming Guru Join Date: Jun 2005 Location: elemental plane Posts: 1,429 Rep Power: 5	LOL, I'm totally clueless, so I will just follow anybody that sounds credible here I really dont care about it anyway. __________________ "Employ your time in improving yourself by other men's writings, so that you shall gain easily what others have labored hard for." -- Socrates

Nov 26th, 2005, 10:17 PM	#9
nnxion Programming Guru Join Date: Jun 2005 Location: elemental plane Posts: 1,429 Rep Power: 5	One thing I haven't resolved yet is that when using fgets, and the line cannot be put into buffer, then I increase the buffer size, but I want to re-get the line, but fgets has already gotten it out of the stream. How would I read the line from the beginning again? Is there a unfgets like an ungetc? :p __________________ "Employ your time in improving yourself by other men's writings, so that you shall gain easily what others have labored hard for." -- Socrates

Nov 27th, 2005, 12:38 AM	#10
grumpy Programming Guru Join Date: Jun 2005 Location: Adelaide, South Australia Posts: 1,221 Rep Power: 5	OK; 1) Dawei is correct in pointing out that the difference between "i++;" and "++i;" is often insignificant in practice --- and he gives an example, in which there is no difference with his compiler. I would expect most modern compilers would do a similar thing, which makes the difference a moot point. I mentioned it in my earlier post mostly in the interests of completeness (and believing code should be structured around what you intend with no potential for unintended side-effects). 2) There is no unfgets() or conceptual equivalent. I personally would not bother with trying to restart reading the line. With fgets(), a practical clue that the line being read is longer than the buffer would be a lack of a '\n' in the string (as a sanity check in debugging, the buffer probably would be also completely filled eg strlen(buffer) == BUFFER_SIZE - 1, but there is no practical need to rely on that in working code). Then all you need to do is call fgets() again to get more of the line ......

Currently Active Users Viewing This Thread: 1 (0 members and 1 guests)

Thread Tools
Show Printable Version Email this Page
Display Modes
Linear Mode Switch to Hybrid Mode Switch to Threaded Mode

Programming Forums > Application Development > C

realloc not functioning as expected