Reading from socket in C giving weird output

Question

I'm having trouble reading from a socket. The code I'm using is below, sometimes it works just fine, but at other times, it just prints some unreadable characters, or some random readable ones... is there a better way?

    char* answer = (char*) malloc(1024);
    int answerLength = 0;
    char prevChar = 0;
    char newChar = 0;
    while (answerLength < 1024 && read(sock, &newChar, 1) > 0 ) {

            if (newChar == '\n' && prevChar == '\r') {
                    break;
            }
            printf("%d\n", answerLength);
            answer[ answerLength ] = newChar;
            answerLength++;

            prevChar = newChar;
    }
    printf("%s\n", answer);

in C, do not cast the returned value from malloc() (and family of functions). 'magic' numbers like that '1024' should be #defined to greatly ease the burden of debugging and later maintenance. The #define name (go programming style says: all caps) should be indicative of what the 1024 means — user3629249
– user3629249, Commented Apr 29, 2015 at 3:32
regarding this line: 'if (newChar == '\n' && prevChar == '\r') {' the OS/compiler will handle the difference between OS line endings, (some OS use only linefeed, some OS use only carriage return, some OS use a multi char string. The compiler will handle the details, so the code only needs to use '\n' — user3629249
– user3629249, Commented Apr 29, 2015 at 3:36
suggest using calloc() rather than malloc, so the allocated memory is 'already' initialized to all ''\0' so there is no worry about adding a null terminator to the input string. BTW: when answerLength is 1023, should stop, not add another char, as adding the char will result in the 'answer' buffer having no room for the NUL string termination char — user3629249
– user3629249, Commented Apr 29, 2015 at 3:42

Havenard · Accepted Answer · 2015-04-27 21:26:17Z

3

Strings in C must be null-terminated, which means they must have a symbol \0 as the last character.

Since you don't guarantee that it will happen anywhere in your code, answer may be padded with memory garbage next to the data you read.

To make sure it won't happen, use:

answer[answerLength] = '\0';
printf("%s\n", answer);

Also, you could just read() the whole thing straight to answer, you don't need that pointless loop:

int len;
while (len = read(sock, &answer[answerLength], 1024 - answerLength))
    answerLength += len;
answer[answerLength] = '\0';
printf("%s\n", answer);

edited Apr 27, 2015 at 21:26

answered Apr 27, 2015 at 21:22

Havenard

28.1k5 gold badges38 silver badges66 bronze badges

Sign up to request clarification or add additional context in comments.

4 Comments

antiduh Over a year ago

Agreed. It's important to remember that the memory returned from malloc will be filled with undefined contents; it's not zeroed unless the code explicitly zeros it, thus, since the code doesn't manually null terminate, the printf could print any amount of garbage at the end depending solely on what was in that memory previously.

Iharob Al Asimi Over a year ago

Or don't use printf() but a function that takes the length as an argument, the key problem is that printf() needs to determine where the string ends, since it does not find '\0', it keeps reading through the garbage...

caf Over a year ago

@iharob: printf() can take the length as argument: printf(%.*s\n", answerLength, answer)

caf Over a year ago

Your single read() isn't equivalent to the original - the original read up to the first "\r\n" sequence, 1023 characters or error (whichever happens first) , but yours just takes whatever it gets in one read, which might not include a "\r\n" sequence at all, or might include more than one.

Iharob Al Asimi · Accepted Answer · 2015-04-27 21:26:15Z

2

The data you read isn't terminated with a '\0' character, so you can't treat is as a string.

edited Apr 27, 2015 at 21:26

Iharob Al Asimi

53.1k6 gold badges66 silver badges100 bronze badges

answered Apr 27, 2015 at 21:23

Andrew Henle

35.1k3 gold badges29 silver badges62 bronze badges

Comments

Serdalis · Accepted Answer · 2015-04-27 21:38:41Z

1

Your char array is not guaranteed to be null terminated. This means that the printf may print more than just what is in your array since it looks for a null termination to stop outputting characters.

You also don't initialise the allocated memory before using it, which is bad practice since the memory can contain random garbage.

To get the code to work better and hopefully fix your problem, you should do the following:

char* answer = malloc(1024 + 1); /* add extra byte for null terminator */
/* other variables the same */

memset( answer, '\0', 1024 + 1 ); /* initialise memory before use */
while (answerLength < 1024 && read(sock, &newChar, 1) > 0 ) {
    /* loop is the same */
}
printf("%s\n", answer);

There is also an argument in printf which will tell it to print a certain number of characters. Like so:

printf( "%.*s\n", answerLength, answer );

edited Apr 27, 2015 at 21:38

answered Apr 27, 2015 at 21:25

Serdalis

10.5k2 gold badges42 silver badges58 bronze badges

1 Comment

Iharob Al Asimi Over a year ago

Please remove the cast from malloc(), read this

Collectives™ on Stack Overflow

Reading from socket in C giving weird output

3 Answers 3

4 Comments

Comments

1 Comment

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

4 Comments

Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Linked

Related