User-input string into array, causing buffer overflow

Question

I am quite new to C, and I'm working on a program right now where I am asking the user to input character arrays(strings) in the form of program names, store these names in an array then pass this array to another function.

Right now, the latest way I've attempted to achieve this is with a dynamically allocated array, as I don't know what the user may want to input, and here's how I'm doing that:

char *process;
process = (char *)malloc(sizeof(char) * (i+1));

I then go on to request the user's input and confirm their input with printf:

printf("Enter process name: ");
scanf("%79s", &process); // take input, load into array
// fgets(process, sizeof(ARRAY_SIZE), stdin);
printf("You entered: %s\n", process[i]);

I've tried both scanf() and fgets(), where scanf() actually works and fgets() causes the program to jump completely past my printf statement and goes on to this:

for(i = 0; i < ARRAY_SIZE; i++)
    printf("process = %s\n", process[i]);

Which is where the program terminates with a Segmentation Fault. I understand this is due to the user input overrunning the buffer allocated.

In gdb when printing the value of process, it's filled with garbage (which from my understanding is how C initializes an array before values are added). Right after scanf("%79s", &process), process shows my input added at the beginning of the garbage instead of replacing the garbage. My question is how do I "empty" my array so I quit overrunning the buffer? Been at this for three days trying several methods, and I think it has to do with me missing some detail of how to properly initialize my array. I have #define ARRAY_SIZE 80 which should be more than adequate to get this functioning, though may not suffice in later implementations.

Any help is much appreciated.

You have to tell us what is i and its value but yet scanf("%79s", &process); is wrong and has to be scanf("%79s", process); — ouah
– ouah, Commented Jan 8, 2015 at 22:34
You have bits and pieces of code that don't look right. You should post an MCVE. That will be helpful for everybody to see what and where your code is not quite right. — R Sahu
– R Sahu, Commented Jan 8, 2015 at 22:42
I initialized i as i = ARRAY_SIZE;. Why would I do scanf("%79s", process) as opposed to my original version, though? I thought I would want to load the input at the beginning address of process. I'll pull together an MCVE for you guys, as I have several other functions that work fine in the program but make it quite long. — TomJ
– TomJ, Commented Jan 8, 2015 at 22:46
K&R 2nd. ed is ANSI C89/ISO C90 compliant and those are a subset of ISO C99 and C11. C89 is the last standard that is defined by ANSI rather than ISO. If you are using the 1978 1st edition of K&R then you really are in trouble, but I doubt that and do not see what that preamble has to do with your question in any case? — Clifford
– Clifford, Commented Jan 8, 2015 at 22:51
@TomJ scanf("%79s", &process) is writing in the process object not in the buffer process is pointing to. — ouah
– ouah, Commented Jan 8, 2015 at 22:52

Clifford · Accepted Answer · 2015-01-08 23:25:35Z

2

The variable process is already a pointer, &process has type char** and is a pointer to the pointer variable, not the pointer to the allocated buffer.

sizeof(char) always equals 1 by definition, so is unnecessary in the malloc() call. If you use scanf() it should be thus:

scanf( "%79s", process ) ;

but because the length is hard-coded in the string and not related to either i or ARRAY_SIZE it is better to use fgets(), but you use a length argument sizeof(ARRAY_LEN) which is the same as sizeof(int); you meant just ARRAY_LEN. In this case the the allocation need not have been i + 1 because fgets() inserts the nul within the size given, so:

fgets( process, ARRAY_SIZE, stdin ) ;

Moreover it is hardly necessary to use malloc() for a fixed length array; you are not setting the array to fit the input, but restricting the input to fit the array. That being the case then I would suggest:

char process[ARRAY_SIZE] ;
printf( "Enter process name: " ) ;
fgets( process, sizeof(process), stdin ) ;

For the output of the input data character-by-character, you need the %c format specifier, not %s:

for( i = 0; i < ARRAY_SIZE; i++ )
    printf( "process = %c\n", process[i] ) ;

but that will print unused, unitialised characters from the array, so more likely:

for( i = 0; process[i] != '\0'; i++ )
    printf( "process = %c\n", process[i] ) ;

or since process is a nul terminated string, simply:

printf("process = %s\n", process ) ;

So ultimately all you need is:

char process[ARRAY_SIZE] ;
printf( "Enter process name: " ) ;
fgets( process, sizeof(process), stdin ) ;
printf("process = %s\n", process ) ;

edited Jan 8, 2015 at 23:25

answered Jan 8, 2015 at 23:12

Clifford

94.3k14 gold badges93 silver badges176 bronze badges

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

1 Comment

TomJ Over a year ago

beautiful. that worked like a charm. I was having a misunderstanding in how to actually get the data from stdin to the array, and that answered it perfectly. Thanks @Clifford, you clarified a lot for me.

BLUEPIXY · Accepted Answer · 2015-01-08 23:03:35Z

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

int main() {
    char *process;
    int i = 0;//number of input character;
    size_t size;//Allocated size
    int ch;

    //process = (char *)malloc(sizeof(char) * (i+1));//+1 for '\0'
    size = 16;//Initial size
    process = malloc(size);//cast don't need in C

    printf("Enter process name: ");//fflush(stdout);
    while((ch = getchar())!=EOF && ch != '\n'){
        if(i == size -1){//-1 for NUL('\0')
            char *temp = realloc(process, size += 16);//expand size
            if(temp==NULL){
                fprintf(stderr, "failed to malloc\n");
                free(process);
                exit(EXIT_FAILURE);
            }
            process = temp;//successfully extended update
        }
        process[i++] = ch;
    }
    process[i] = 0;
    printf("process = %s\n", process);//type of process[i] is `char`
    free(process);
    return 0;
}

Collectives™ on Stack Overflow

User-input string into array, causing buffer overflow

2 Answers 2

1 Comment

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

1 Comment

Comments

Your Answer

Sign up or log in

Post as a guest

Related