1

I need getline() to read the request header sent by my browser to the webserver I'm programming. This is the getMessage function which is supposed to do that task:

char *getMessage(int fd) {
  FILE *sstream = fdopen(fd, "r");
  // initialise block to 1 char and set it to null
  char *block = malloc(sizeof(char));
  *block = '\0';
  int size = 1;

  // Read from the file descriptor fd (using a FILE stream) until a blank line is
  // received.
  // Read 100 lines (buffersize) from sstream and put into the buffer. If lines have
  // been successfully read concatenate them with block.
  int buffersize = 100;
  char *buffer = malloc (buffersize + 1);

  while(getline(&buffer,&buffersize,sstream) != -1){
     int length = strlen(buffer);
     printf("Buffer length: %d\n",length);
     block = realloc(block,strlen(block)+strlen(buffer)+1);
     strcat(block,buffer);
     if(strcmp(buffer,"\r\n") == 0) break;
 }

  int len = strlen(block);
  printf("Block length: %d\n", len);
  printf("%s \n", block);
  return block;
} 

Basically the input of the getMessage function (fd), is the input from my listening socket declared in my main method. I have verified that the output is correct. Now I need to convert the output from the file descriptor to a string and return that string. But every time I run my server it gets stuck in the while loop. Not executing the statements in the loop. EDIT: Added a loop-terminating condition: Now it jumps to "Block length" immediatley. Help is much appreciated!

Abhischek
  • 189
  • 2
  • 7
  • 18

2 Answers2

5

If you are using the POSIX 2008 getline() function, then you're throwing away useful information (it returns the length of the line it reads, so if you capture that information, you would not need the strlen() in the loop.

If the code blocks on a getline() call, it probably means that the upstream socket is not closed, but there is no data being sent any more. Your sending code needs to close the socket so that this code can detect EOF.

Or, since you discuss 'a blank line', then maybe your code should be checking for a line containing just \r\n (or maybe just \n) and break the loop; your code is not doing that at the moment.

Your loop also exhibits quadratic behaviour because you are repeatedly using strcat(). You would do better to keep tabs on the end of the string and simply strcpy() the new data after the old, then adjust the pointer to the end of the string.


On further review, I note that you use fdopen() to open a file stream based on the file descriptor, but you neither close it nor return the file stream to the caller for closing. This leads to a leakage problem.

Rule of Thumb: if you allocate a resource, you should release it, or pass it back to be released.

I recommend changing the interface to use an already-open FILE *, and doing the fdopen() in the calling code. Alternatively, if you won't need the file descriptor again, you can keep the current interface and use fclose() before returning, but this will close the underlying file descriptor too.

This code works for me (MacOS X 10.7.2; XCode 4.2.1):

#include <unistd.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

extern char *getMessage(FILE *);

char *getMessage(FILE *fp)
{
    char    *block = 0;
    size_t   size = 0;
    size_t   buffersize = 0;
    char    *buffer = 0;
    ssize_t  newlen;

    while ((newlen = getline(&buffer, &buffersize, fp)) > 0)
    {
        printf("Buffer length: %ld\n", (long)newlen);
        block = realloc(block, size + newlen + 1);
        strcat(&block[size], buffer);
        size += newlen;
        if (strcmp(buffer, "\r\n") == 0)
            break;
    }

    printf("Block length: %zd\n", size);
    if (size > 0)
        printf("<<%s>>\n", block);
    return block;
}

int main(void)
{
    char *msg;
    while ((msg = getMessage(stdin)) != 0)
    {
        printf("Double check: <<%s>>\n", msg);
        free(msg);
    }
    return 0;
}

I tested it with a file with DOS-style line endings as standard input, with both a blank line as the last line and with a non-blank line. Two blank lines in a row also seemed to be OK.

Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278
  • Hi added the strcmp statement to break out of the loop. Still no change! Yes as you said the spec says to stop reading once I receive the \r\n - end of transmission. But even adding the break changes nothing. No the loop breaks and I get "Block length: 0" as a result. – Abhischek Nov 26 '11 at 18:17
  • The problem @codaddict points out is also valid - and your compiler should be telling you about it in no uncertain terms. If it isn't, you haven't got enough warnings turned on. – Jonathan Leffler Nov 26 '11 at 18:21
  • Ugh! I note that the `realloc()` code has the `space = realloc(space, new_size)` memory-leaking anti-pattern in it, and no error checking. Repeated `strcat()` is sub-optimal (leads to quadratic behaviour). I should find the person who wrote this and get them to fix it … oh … well, I should fix it. It works on toy samples with no errors. – Jonathan Leffler Feb 16 '15 at 00:25
5
char buffer = (char *) malloc (buffersize + 1);

should be:

char *buffer = malloc (buffersize + 1);
codaddict
  • 445,704
  • 82
  • 492
  • 529