-2

I'm writing a socket program and trying to parse command line arguments from the user. Let's say I have a function that looks like this:

void *parseUserReq(char *arg)
{
  int sock;

  char buffer[1024];

  int readIn;

  char *str1, *str2, *str3;


  //cast sock back to int
  sock = (int)arg;

  //Get input from client
  readIn = recv(sock, buffer, 1024, 0);
  buffer[readIn] = '\0';

  //parse the 3 strings the user is supposed to enter

  str1 = (char*)malloc(sizeof(buffer)+1);
  strcpy(str1, buffer);         //  copy header data into str1

  str1 = strtok(str1, " ");
  printf("%s\n", str1);

  str2 = strtok(str2, " ");
  printf("%s\n", str2);

  str3 = strtok(str3, "\r\n");
  printf("%s\n", str3);

} // End of parseUserReq

How would I call that function in main? Currently I'm doing something like this:

int main(int argc, char** argv)
{
    // ^^^Calls for create, bind, listen, accept, and send^^^


    char buffer[1024];
    parseUserReq(buffer); // segmentation fault
    return 0;
}

I can add my code for creating and binding the socket, and listening, accepting,sending if necessary.

EDIT: Just spent a while on this, and found that the issue is more complex than I initially described. I will use Valgrind to identify the source of the problem, or just rewrite the function entirely in accordance with your commentary. I'll hunt around for answers more extensively before posting next time.

Thanks again for the help everyone!

  • Hint: The visible part of your code is correct. – class stacker Feb 25 '16 at 19:04
  • Edit: added the relevant code for the function body, parameters, and the buffer array in the main() function. – lord_missingno Feb 25 '16 at 19:48
  • Your compiler should warn already for not returning a result from a non-`void` function! – too honest for this site Feb 25 '16 at 20:56
  • You're casting the receive buffer to int and treating it as a socket file descriptor, which it isn't. `parseUserReq(char *arg)` probably needs to be something like `parseUserReq(int sockfd, char *buf)`. – jjlin Feb 25 '16 at 20:56
  • There are various faults in your code which can invoke undefined behaviour. Using `char` for a raw data buffer is a bad idea. Use `unsigned char` at least. Also don't cast the result of `malloc` & friends in C or `void *` in general. – too honest for this site Feb 25 '16 at 20:58
  • On which operating system? You'll better define (for readability reasons) a `typedef` for function pointer signatures, e.g. like [here](http://stackoverflow.com/a/9143434/841108) – Basile Starynkevitch Feb 27 '16 at 07:14

2 Answers2

1
str1 = (char*)malloc(sizeof(buffer)+1);
strcpy(str1, buffer);         //  copy header data into str1

recv didn't null-terminate buffer. recv returns the number of bytes received. Therefore:

str1 = (char*)malloc(readIn);
memcpy(str1, buffer, readIn); //  copy header data into str1
str1[readIn] = 0;

Incidentally, recv probably failed due to

sock = (int)arg;

You need to pass the buffer and socket as two different arguments.

You are about to find out there's a host of problems with this model but something's gotta run so you can learn it.

Joshua
  • 40,822
  • 8
  • 72
  • 132
0

You declared your function to return a void POINTER not only void which you probably meant.

Either redefine your function or, if you really want a pointer back, declare a void pointer where it's called and assign the return value to that pointer.

In the answer I assume that within the function everything is fine.

  • The OP says he's getting a seg fault. Obviously not due to the function's return type or value, right? – class stacker Feb 25 '16 at 19:07
  • @ClassStacker: Not returning a value from a function which is supposed to is _undefined behaviour_. Which means ther might be a segfault, the computer jumps out of the window or nasal daemons appear. – too honest for this site Feb 25 '16 at 21:03
  • @Olaf Sorry but that's plain wrong. Not specifying the return value of a function which is supposed to return a value leads to an _undefined return value_, not _undefined behaviour_ per se. Since the return value is ignored in the OP's code, it does not at all contribute to the problem he was facing. So please reconsider my comment: "Seg fault obviously not due to the functions's return type or value." It's certainly good to point the OP to the return type/value issue. But more valuable was my comment that the originally posted code was okay, since it was, and it made him edit his Q. – class stacker Feb 26 '16 at 08:05
  • @ClassStacker: Can you provide a reference to the standard? – too honest for this site Feb 26 '16 at 12:02
  • @Olaf As you probably know, the wording in the standard uses the words "undefined behaviour", but with the essential addition "in the value-returning function". Don't you think that if this would lead to a seg fault, the compiler would throw an error instead of a warning? Show me a system where you can provoke a seg fault by not returning a value from a function when that value isn't used by the calling code. I'm willing to bet there's no such system on earth. Apart from that, the key point is that this answer here is entirely besides the point at the time it was written and I commented it. – class stacker Feb 26 '16 at 12:12
  • @ClassStacker "Don't you think that if this would lead to a seg fault, the compiler would throw an error instead of a warning" - No. "The" (actually some) compiler shows warnings for other cases of UB, too. The standard requires only for few cases to generate a _diagnostic_ message, see also 3.4.3p2. It does not use the terms "error" or "warning" in this context. Please don't argue with examples, but stick to the standard only. There are much more than Windows, OS-X, Linux or Unix supported by C. The C standard intentionally does not require UB to result in segfaults. – too honest for this site Feb 26 '16 at 12:32
  • @ClassStacker: And UB can very well cause a program to missbehave much later during execution. That's what the word "undefined" implies. – too honest for this site Feb 26 '16 at 12:34
  • @Olaf I can't see how this relates to the original Q and this A. And thank you, I have used C on platforms you'll most likely not even have the chance to get to know. You're relating to the standard only in the way you choose to. If there were a compiler consciouisly generating code that crashes -- it wouldn't have much of a future, no matter whether it's not _required_ by the standard to generate an error. This A here is completely irrelevant for the OP, even though he shouldn't have written the code that way, and I wasn't making any other point. My last comment on this. See you elsewhere. – class stacker Feb 26 '16 at 13:34