Clarification with C-style Strings

I've managed to load all of the text from a file into a single string. But I'm not sure if the way I've handled it is very safe, or very memory efficient. I'm still learning how all this stuff works.

Code:

...

while((nextChar = fgetc(file)) != EOF)
{
   fileData = strConcat(fileData, (char *)&nextChar);
}
...

char *strConcat(char *str1, char *str2)
{
   size_t size = strlen(str1) + strlen(str2) + 1;
   char *newStr = (char *)malloc(size);

   sprintf_s(newStr, size, "%s%s", str1, str2);

   return newStr; 
}

I'm mostly worried about my use of malloc. But everything seemed to be working ok so I moved on and then I attempted to write some code to search through this char array for specific characters.

Code:

for (i = 0; i < strlen(fileData); i++)
{
   if (*(fileData + i) == '\n')
      printf("New line at: %i\n", i);
}

And this seemed to have worked correctly. What I want to do now is assign a char pointer to the start character of each new line. But I can't seem to figure out how to do it. I'll need a dynamically sized array but I can't work out how to make one.

One way you may want to think about is using pointers to pointers (**char) which point at the elements in the string array which point to the newline characters. This would make it pretty easy to access, and then continue to access the elements in the next line.

I've sort of got it working. Here's the code I have:

Code:

// Includes
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

// Prototypes
char *strConcat(char *str1, char *str2);

// Main entry point
int main(int argc, char *argv[])
{
	char *fileData = "";
	char **lineData;
	int nextChar, lineCount = 0, i;
	FILE *file;

	if (argc != 2)
	{
		printf("Usage: &#37;s filename", argv[0]);
		return 0;
	}

	file = fopen(argv[1], "r");

	if (file == 0)
	{
		printf("File not found!");
	}
	else
	{
		while((nextChar = fgetc(file)) != EOF)
		{
			if (nextChar == '\n')
				lineCount++;
			fileData = strConcat(fileData, (char *)&nextChar);
		}
	}

	fclose(file);

	printf("%s\n\n", fileData);

	lineData = (char **)malloc(sizeof(char) * lineCount);
	lineCount = 1;

	lineData = &fileData;

	for (i = 0; i < strlen(fileData); i++)
	{
		if (*(fileData + i) == '\n')
		{
			*(*lineData + lineCount) = *(fileData + i + 1);
			lineCount++;
		}
	}

	for (i = 0; i < lineCount; i++)
	{
		printf("Line: %i - Char: %c\n", i, *(*lineData + i));
	}

	getchar();

	return 0;
}

char *strConcat(char *str1, char *str2)
{
	size_t size = strlen(str1) + strlen(str2) + 1;
	char *newStr = (char *)malloc(sizeof(char) * size);

	sprintf_s(newStr, size, "%s%s", str1, str2);

	return newStr; 
}

There seems to be some overwriting in data. When I print out the fileData string at the very end, there's some new characters in it. Here's my test file data.

Code:

_                 <
 _ $33>100>_   114^
 _<72<101<<108>^
_.          /^/<111>\87\>\32\>
&{{{{{{{{{{{{#
, Output: Hello World!

1)

A pointer to a character is NOT a string. Repeat several times until you understand. You can't just take a pointer to the character you received from fgetc(), pass it to a string function (like strlen()), and expect it to work.

2)

Code:

while((nextChar = fgetc(file)) != EOF)
{
	if (nextChar == '\n')
		lineCount++;
	fileData = strConcat(fileData, (char *)&nextChar);
}

You are leaking memory like crazy here. You don't even attempt to free the previous value of fileData before you overwrite it.

Suggestion: Why don't you read the file line-by-line using fgets() instead? Oh, and there's no reason why you need to allocate a new chunk of memory every single time you concatenate a string. There's a strcat() function for a reason. Allocate a sufficiently large amount of memory to start with (say.. 500 chars or so), and realloc only when you need to.

Thanks, that was exactly the kind of information I was after regarding my file reading and string concatenation.

I fully understand that a pointer to a character is not a string. (How could I have possibly written (*(fileData + i) == '\n') if i didn't?) What am I meant to say? the 'fileData character pointer which is pointing to the first character of a string of characters all sequential in memory?'. I think 'fileData string' is a little easier to say and I'm quite sure most people here would understand the meaning of it.

No one's complaining about fileData. We are complaining very strenuously about nextChar. nextChar is not null-terminated, which means you will get, on average, 2 billion extra characters after your character.

I'm not sure what you're on about there in the second half of that post, but my first point was in regards to this:

Code:

...
		while((nextChar = fgetc(file)) != EOF)
		{
			if (nextChar == '\n')
				lineCount++;
			fileData = strConcat(fileData, (char *)&nextChar);
		}
...
char *strConcat(char *str1, char *str2)
{
	size_t size = strlen(str1) + strlen(str2) + 1; //str2 is not a string!
	char *newStr = (char *)malloc(sizeof(char) * size);

	sprintf_s(newStr, size, "%s%s", str1, str2); //str2 is still not a string

	return newStr; 
}

It had nothing to do with anything you said.

Is that clearer?

Ah now I see what you're saying. But I'm not sure how I'd go about terminating that character? Haven't I declared nextChar int? As required by the function fgetc()?

Since strConcat is your own concoction, you can just change it to take the second parameter as a char (and printf a %c, instead of a %s, and so on).