CODEWITHSUNDEEP
carraysstringpointersstrcpy
Kevin Price
Kevin Price

Reputation: 11

how to add String by value and not reference

Im making a really simple TODOlist in C.

My add function takes a char * as its parameter. When I add it to my char ** list of reminders, it adds the memory address of my buffer instead of the value of the string.

The problem becomes apparent when I run the given source below.

If you try to [a]dd a string, say "Test", and then issue command [p]rint, a "p" will be printed.

I understand that this is because my list[0] is holding a pointer to my buffer, so that when the value of buffer changes, so does the value in my list.

My C is rusty and I know that strcpy() might solve this issue? But is this the usual way to handle this?

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


int size = 0;
int capacity = 50;
char **list;

void initList() {
    list = malloc( capacity * sizeof( char * ) );
    for (int i = 0; i < capacity; i++ ) {
        list[i] = malloc( 256 * sizeof( char ) ); 
    }
}

void saveReminders() {  
}

void add( char *reminder ) {
    list[size++] = reminder;
}

void delete( int index ) {
}

void print() {
    for ( int i = 0; i < size; i++ )
        printf( "%s\n", list[i] );
}

void quit() {
    saveReminders();
    exit( 0 );
}

void readReminders() {
}

void helpMenu() {
    printf( "Enter a command:\n" );
    printf( "[a]dd a reminder\n" );
    printf( "[d]elete a reminder\n" );
    printf( "[p]rint all reminders\n" );
    printf( "[q]uit\n" );
}

void menu() {
    helpMenu();

    while ( 1 ) {
        char buffer[64];
        int index = 0;
        printf( "> " );
        scanf( "%s", buffer );

        if ( strcmp( buffer, "a" ) == 0 ) {
            printf( "Enter a reminder:\n> " ); 
            scanf( "%s", buffer );
            add( buffer );
            helpMenu();
        }else if ( strcmp( buffer, "d" ) == 0 ) {
            printf("Remove which index?\n> " );
            scanf( "%d", &index );
            delete( index );
            helpMenu();
        }else if ( strcmp( buffer, "p" ) == 0 ) {
            print(); 
            helpMenu();
        }else if ( strcmp( buffer, "q" ) == 0 ) {
            quit(); 
        }
    }
}

int main( int argc, char* argv[] ) {
    initList();  
    menu();  
}

Upvotes: 1

Views: 77

Answers (2)

Shlomi Agiv
Shlomi Agiv

Reputation: 1198

instead of:

list[size++] = reminder

use

strcpy(list[size++], reminder)

or better yet, when you scanf, remember the number of characters written, pass it to add(), then do:

strncpy(list[size++], reminder, len)

Upvotes: 0

Sourav Ghosh
Sourav Ghosh

Reputation: 134366

Yes, you thought it right. In your code, instead of assigning the pointer itself, do something like

 strcpy(list[size++], reminder);

to copy the content. See the man page for reference.

If you'd like to have a second approach, you don't need to malloc() for each and every list[i] either. Instead of using malloc() and then strcpy(), you can directly use strdup() and assign the return value to each list[i] to get the same result.

That said, some other points to note

  • Before making use of list[size++], ensure that size is less than capacity, otherwise you'll end up overrunning the allocated memory, creating undefined behavior.
  • scanf( "%s", buffer ); should be scanf( "%63s", buffer ); to prevent any possible buffer overflow.
  • Always check the return value of malloc() for success before using the returned pointer.
  • sizeof( char ) is defined to be 1 in C standard. using it as a multiplier is redundant.

Upvotes: 3

Related Questions