views:

357

answers:

5

Yo!

I'm trying to copy a few chars from a char[] to a char*. I just want the chars from index 6 to (message length - 9).

Maybe the code example will explain my problem more:

char buffer[512] = "GET /testfile.htm HTTP/1.0";
char* filename; // I want *filename to hold only "/testfile.htm"

msgLen = recv(connecting_socket, buffer, 512, 0);
strncpy(filename, buffer+5, msgLen-9);

Any response would help alot!

+10  A: 

I assume you meant...

strncpy(filename, buffer+5, msgLen-9);

The problem is you haven't allocated any memory to hold the characters you're copying. "filename" is a pointer, but it doesn't point at anything.

Either just declare

char filename[512];

or malloc some memory for the new name (and don't forget to free() it...)

There are a few problems with the use of strncpy() in your code.

  • buffer+5 points to the sixth character in string (the "T"), while you said you wanted the backslash.
  • The last parameter is the maximum number of bytes to copy, so should probably be msglen-13.
  • strncpy() won't null terminate the copied string, so you need to do that manually.
  • Also, from a readabilty perspective, I prefer

    strncpy(filename, &buffer[4], msgLen-(9 + 4));

&buffer[5] is the address of the character at the fifth position in the array. That's a personal thing, though.

Also, worth pointing out that the result of "recv" could be one byte or 512 bytes. It won't just read a line. You should really loop calling recv until you have a complete line to work with.

Roddy
Thanks!Actually, when I usestrncpy(filename, ,filename contains "/testfile.htm H", but when I usestrncpy(filename, it works perfectly.Thanks again!
petsson
@petsson - That's probably because your "recv()" call is returning a string with a newline sequence at the end. You should really have a more robust means of detecting the start and end of the filename.
Roddy
Yes, Roddy's and my answer relied on the fact that you enter the exact format from the socket, in a *single* receive call.
Mehrdad Afshari
+5  A: 

First of all you should allocate a buffer for filename. The next problem is your offset.

char buffer[512] = "GET /testfile.htm HTTP/1.0";
char filename[512]; // I want *filename to hold only "/testfile.htm"

msgLen = recv(connecting_socket, buffer, 512, 0);
strncpy(filename, buffer+4, msgLen-4-9); 
//the first parameter should be buffer+4, not 5. Indexes are zero based.
//the second parameter is count, not the end pointer. You should subtract
//the first 4 chars too.

Also you should make sure you add a null at the end of string as strncpy doesn't do it.

filename[msgLen-4-9] = 0;

You could also use memcpy instead of strncpy as you want to just copy some bytes:

memcpy(filename, buffer+4, msgLen-4-9);
fileName[msgLen-4-9] = 0;

In either case, make sure you validate your input. You might receive invalid input from the socket.

Mehrdad Afshari
Won't the strncpy also not have a null terminator? He should also add that to the end with filename[msgLen-4-8] = '\0';
Paul Tomblin
:)) Lol! I was just editing the question to add this note. When I posted it, I noticed your comment! What a timing!
Mehrdad Afshari
;-) I saw Pauls comment and had just added that to my answer... oops.
Roddy
+1  A: 

Your example code has the line:

char* filename;

This is an uninitialised pointer - it points nowhere, and isn't backed by any storage. You need to allocate some memory for it, e.g. using malloc() (and remember to free() it when you're done), or, in this case, you can probably simply declare it as a character array, e.g.

char filename[SOME_BUFFER_SIZE];

Declaring an array on the stack has the advantage that you don't need to explicitly free it up when you're done with it.

Fundamentally, arrays in C are just syntactic sugar that hide pointers, so you can (usually) treat a char[] as a char*.

Rob
+1  A: 

You've not allocated any space for the filename

Either replace the declaration of filename with something like

char filename[512]

or (probably better) allow enough space for the filename

filename = (char *)malloc(msgLen - 9  - 6 + 1 ); /* + 1 for the terminating null */
Paul
You don't need to write (char *)malloc. malloc is sufficient. But guess it's never bad to specifiy :)
Filip Ekberg
Yeah, I prefer being explicit about such things :)
Paul
A: 

You have not allocated any memory space to filename yet. It is a pointer... but until initialized, it is pointing to some random area of memory.

You need to allocate memory for filename. As Roddy said, you could declare filename to be 512 bytes. Or you could:

filename = (char*)malloc(512*sizeof(char));

(note: sizeof(char) isn't strictly needed, but I think helps clarify exactly what is being allocated).

After this statement, filename is a pointer to allocated memory, you are free to use it, including copying data to it from buffer. If you copy only a limited region, be sure you leave filename null-terminated.

abelenky
don't forget to free(filename) afterwards...
lImbus