Thread: Functions for C strings

Originally Posted by officedog

Code:

void getUserInputLine(FILE * fp, char * ps) {
    char buf[100];
    if ( ((ps = fgets(buf, sizeof(buf), fp))) != NULL) {
        printf("\n%s", buf);
        printf("\n%s", ps);
    }
}

int main (int argc, const char * argv[]) {
    char * pString;
    getUserInputLine(stdin, pString);
    return 0;
}

That does absolutely nothing. The pointer is passed to the function by value, so the assignment 'ps = ' is applied to the copy, not the variable declared within the scope of main. To get all that to work would require passing a char**.

Thanks for replies. This a great forum, very helpful. I remember how painful arrays and array pointers are! I know this is all really basic stuff, and I appreciate the help to remember some of it.

I try and make a summary of this (please correct if this is wrong)

If declare a char array and pass the array to the function, this will effectively be passed by reference as the name of the array is the same as the address of array[0]. Although the function copies the address, the address still refers to the array which was created outside the function. The function can therefore modify the array and the modifications will exist outside the function.

I'm still more confused about the char * and why it is a problem passing it into a function - I'll have to read up on this.

So my minimal template function now looks like this.

Code:

void getUserInputLine(char buf[ ], int bufSize) {
    if ( ((fgets(buf, bufSize, stdin))) != NULL) {
        printf("\n%s", buf);
    }
    //strchr function here
}

int main (int argc, const char * argv[]) {
    int bufSize = 100;
    char buf[bufSize];
    getUserInputLine(buf, bufSize);
    printf("\nbuf[0] = %c", buf[0]); //tests result
    return 0;
}

A downside of this is being restricted to the self imposed buffer size. What views are there on the use of malloc and how it compares to passing a char array? I'm trying to get a grasp of objective C at the same time, so returning an NSString is also an option...

Just to add a progress notes:

I have read up on the difference between
char array[] = "a string";
and
char * pString = "a string"

Some of the things I read were
a) "a string" is a string literal / string constant and is stored in protected/read only memory
b) char array[] makes a copy of the string literal on the stack and can be altered
c) char * aString simply holds the address of the string literal. The string cannot be altered using for example aString[0] = 's'
d) sizeof(array) will return the size of the array, but cannot do this if the array is passed into a function - in this case 'array' will be treated as a char *.

Current progress with my generic function looks like this. I returned to fgetc as I like its simplicity and am getting a better feel for how it works. Would be interested to know if anyone can spot any problems with this. It seems to work OK I think

Code:

//Initialise all elements of array to '\0'
//Note - best to pass array size using sizeof(charArray)
void GC_InitCharArray(char charArray[], int arraySize) {
    int index;
    for (index = 0 ; index < arraySize ; index++) {
        charArray[index] = '\0';
    }
}

void GC_GetUserInput(char buf[], int bufSize) {
    int index = 0;
    int c;
    //initialise array so all elements are '\0'
    GC_InitCharArray(buf, bufSize);
    
    //while fgetc(stdin) ! = '\n' designed to clear input buffer
    while ( (c = fgetc(stdin) ) != '\n') {
        
        //if there is space in array, add character
        // bufSize-1 is sentry value to leave a terminating '\0'
        if (index < (bufSize - 1) ) {
            buf[index] = c;
        }
        //for debugging
        else if (index == bufSize -1) {
            printf("\nBuffer full");
        }
        index++;
    }
    
    //if all characters from input stream have been read, replace the terminating '\n' with '\0'
    if (c == '\n') {
        buf[index] = '\0';
    }
}
    

//MAIN
int main (int argc, const char * argv[]) {
    int bufSize = 5;
    char buf[bufSize];
    
    GC_GetUserInput(buf, sizeof(buf));
    printf("\n%s...", buf);
       
    return 0;
}

Originally Posted by officedog

Thanks for pointing this out bithub, I appreciate your help. Odd how such a simple loop can hurt the brain. I think you are right and have modified the code but putting index++ into the initial if(...) statement.

When I think more about it, the problem seems to be that, because the while loop always reads through the stdin stream, the final statement will always evaluate to true and will be executed. I put it there, thinking I needed to have something that replaced the '\n' with '\0'. However, because the char array is initialised to '\0' AND the initial while loop exits on '\n', there is actually no need for it. I think this is right anyway. I'll post the current version without all the printf messages and comments

Code:

void GC_GetUserInput(char buf[], int bufSize) {
    int index = 0;
    int c;
    
    GC_InitCharArray(buf, bufSize);
    
    while ( (c = fgetc(stdin) ) != '\n') {
        if (index < (bufSize - 1) ) {
            buf[index] = c;
            index++;  
        }
    }
}

EDIT: was just wondering if assigning an int to a char array will always work, or whether I should convert it before assignment?

Don't forget to null-terminate the string after the loop (you can use 'index' as the position, obviously).

>> GC_InitCharArray

For strings, this really isn't necessary. Just set the first character to 0 and you're good to go.

I'm still more confused about the char * and why it is a problem passing it into a function - I'll have to read up on this.

Think of it this way. A pointer is really just an integer whose value is an address. So naturally, passing one to a function allows you to read/write to the data at that address. But the pointer itself is passed by value, so you can't alter the address it contains from the caller's standpoint, since you just have a copy of it. In order to do so requires passing a pointer to a pointer, eg:

Code:

/*
    Only modifies the copy of the pointer
*/
void foo( char* ptr )
{
    ptr = "Unread message";
}

/*
    Modifies the caller's pointer
*/
void bar( char** ptr )
{
    *ptr = "New message";
}

int main( void )
{
    char* str = "Message";
    printf( "'str': %s\n", str );
    foo( str );
    printf( "'str': %s\n", str );
    bar( &str );
    printf( "'str': %s\n", str );
    return 0;
}

I have worked further on my efforts at making a function which dynamically allocates memory to store a string from user input. This time I have added an extra bit so it will resize (I hope) depending on the input from the user. It seems to work, but this is new territory for me. Can anyone see problems with this?

Code:

char * GC_GetUserInputDyanmic() {
    int baseSize = 2;  //set to very small level for test runs
    char c;
    
    char * spDynamic = malloc(sizeof(char) * baseSize);
    if (spDynamic == NULL) {
        return spDynamic;
    }
    
    int index = 0;
    
    while ((c = fgetc(stdin)) != '\n'  && c != EOF) {
        if(index < (baseSize - 1)) {
            spDynamic[index] = c;
            index++;
        }
        else {
            printf("\nReallocating space: baseSize = %d, index = %d", 
                   baseSize, index);
            baseSize = baseSize * 2;
            realloc(spDynamic, sizeof(char) * baseSize);
            spDynamic[index] = c;
            index++;
        }
    }
    spDynamic[index] = '\0';
    
    return spDynamic;
}

>> realloc(spDynamic, sizeof(char) * baseSize);

Again, remember that in C all parameters are passed by value, and so all that realloc gets is a *copy* of your pointer - there is no way for it to modify the caller's pointer (eg: set it to the new block of memory). Fortunately, realloc knows this, and conveniently returns a pointer to the new block (or NULL, if unavailable).

Other than that, though, I'd say the function looks fine.

One suggestion, though. Look at this part of the loop:

Code:

            spDynamic[index] = c;
            index++;

Instead of writing that twice, you can simply perform the size check first, resize if necessary, and then at the bottom of the loop (outside of any conditional) have the assignment/increment (the above code fragment, that is).

Thank you for this feedback. It's helping a lot to build some confidence

I rewrote the function with the larger temporary storage, rather than resizing with realloc. It still seems to work OK. I decided to keep the option of resizing, just in case the 'user' has a lot to say! But with such a large buffer, it probably won't be used. And then tidied up the while loop.

Code:

char * GC_GetUserInputDyanmicV2() {
    int baseSize =  2048;
    char c;
    
    char * spDynamic = malloc(sizeof(char) * baseSize);
    if (spDynamic == NULL) {
        return spDynamic;
    }
    
    int index = 0;
    
    while ((c = fgetc(stdin)) != '\n'  && c != EOF) {
        if (index > baseSize -2) {
            printf("\nReallocating space: baseSize = %d, index = %d", 
                   baseSize, index);
            baseSize += baseSize;
            realloc(spDynamic, sizeof(char) * baseSize);
            
        }
        spDynamic[index] = c;
        index++;
    
    }
    spDynamic[index] = '\0';
    
    int len = strlen(spDynamic);
    
    char * stringReturn = malloc(sizeof(char) * (len + 1) );
    strncpy(stringReturn, spDynamic, len+1);
    stringReturn[len] = '\0';
    
    free(spDynamic);
    spDynamic = NULL;
    
    return stringReturn;
}

>> realloc(spDynamic, sizeof(char) * baseSize);

You're still not capturing the return value correctly.

OK I understand now. Thanks for keeping an eye on this, sebastiani. Really appreciate it. So, the reason it worked fine was because my call to realloc didn't need to find a block of memory somewhere else and could simply extend the existing block.... meaning the pointer was still valid.

Code:

buffer = realloc(buffer, sizeof(char) * baseSize);

I guess I should also add a test for a NULL pointer too