Appending Characters to an Empty String in C

Question

I'm relatively new to C, so any help understanding what's going on would be awesome!!!

I have a struct called Token that is as follows:

//Token struct
struct Token {
    char type[16];
    char value[1024];
};

I am trying to read from a file and append characters read from the file into Token.value like so:

    struct Token newToken;
    char ch;
    ch = fgetc(file);
    strncat(newToken.value, &ch, 1);

THIS WORKS!

My problem is that Token.value begins with several values I don't understand, preceding the characters that I appended. When I print the result of newToken.value to the console, I get @�����TheCharactersIWantedToAppend. I could probably figure out a band-aid solution to retroactively remove or work around these characters, but I'd rather not if I don't have to.

In analyzing the � characters, I see them as (in order from index 1-5): \330, \377, \377, \377, \177. I read that \377 is a special character for EOF in C, but also 255 in decimal? Do these values make up a memory address? Am I adding the address to newToken.value by using &ch in strncat? If so, how can I keep them from getting into newToken.value?

Note: I get a segmentation fault if I use strncat(newToken.value, ch, 1) instead of strncat(newToken.value, &ch, 1) (ch vs. &ch).

Answer 1

I'll try to consolidate the answers already given in the comments.

This version of the code uses strncat(), as yours, but solving the problems noted by Nick (we must initialize the target) and Dúthomhas (the second parameter to strncat() must be a string, and not a pointer to a single char) (Yes, a "string" is actually a char[] and the value passed to the function is a char*; but it must point to an array of at least two chars, the last one containing a '\0'.)

Please be aware that strncat(), strncpy() and all related functions are tricky. They don't write more than N chars. But strncpy() only adds the final '\0' to the target string when the source has less than N chars; and strncat() always adds it, even if it the source has exactly N chars or more (edited; thanks, @Clifford).

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

int main() {
    FILE* file = stdin; // fopen("test.txt", "r");
    if (file) {
        struct Token {
            char type[16];
            char value[1024];
        };

        struct Token newToken;
        newToken.value[0] = '\0';                       // A '\0' at the first position means "empty"

        int aux;
        char source[2] = "";                            // A literal "" has a single char with value '\0', but this syntax fills the entire array with '\0's
        while ((aux = fgetc(file)) != EOF) {
            source[0] = (char)aux;
            strncat(newToken.value, source, 1);         // This appends AT MOST 1 CHAR (and always adds a final '\0')
        }

        strncat(newToken.value, "", 1);                 // As the source string is empty, it just adds a final '\0' (superfluous in this case)
        printf(newToken.value);
    }
    return 0;
}

This other version uses an index variable and writes each singe char directly into the "current" position of the target string, without using strncat(). I think is simpler and more secure, because it doesn't mix the confusing semantics of single chars and strings.

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

int main() {
    FILE* file = stdin; // fopen("test.txt", "r");
    if (file) {
        struct Token {
            int index = 0;
            char type[16];
            char value[1024];        // Max size is 1023 chars + '\0'
        };

        struct Token newToken;
        newToken.value[0] = '\0';    // A '\0' at the first position means "empty". This is not really necessary anymore

        int aux;
        while ((aux = fgetc(file)) != EOF)
            // Index will stop BEFORE 1024-1 (value[1022] will be the last "real" char, leaving space for a final '\0')
            if (newToken.index < sizeof newToken.value -1)
                newToken.value[newToken.index++] = (char)aux;

        newToken.value[newToken.index++] = '\0';
        printf(newToken.value);
    }
    return 0;
}

Edited: fgetc() returns an int and we should check for EOF before casting it to a char (thanks, @chqrlie).

Answer 2

You are appending string that is not initialised, so can contain anything. The end I'd a string is indicated by a NUL(0) character, and in your example there happened to be one after 6 bytes, but there need not be any within the value array, so the code is seriously flawed, and will result in non-deterministic behaviour.

You need to initialise the newToken instance to empty string. For example:

struct Token newToken = { "", "" } ;

or to zero initialise the whole structure:

struct Token newToken = { 0 } ;

The point is that C does not initialise non-static objects without an explicit initialiser.

Furthermore using strncat() is very inefficient and has non-deterministic execution time that depends on the length of the destination string (see https://www.joelonsoftware.com/2001/12/11/back-to-basics/).

In this case you would do better to maintain a count of the number of characters added, and write the character and terminator directly to the array. For example:

size_t index ;
int ch = 0 ;

do
{
    ch = fgetc(file);
    if( ch != EOF ) 
    {
        newToken.value[index] = (char)ch ;
        index++ ;
        newToken.value[index] = '\0' ;
    }
} while( ch != EOF && 
         index < size of(newToken.value) - 1 ) ;