0

In the following code, the printed result for char array dir is gibberish. However, if I comment out the indicated printf statement, the printed result is intelligible. What is going on here? Thanks. sing code blocks / gcc.

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

char* handle_input(int argc,char *argv[]){
    char dir[200];
    printf("Number of arguments: %d\n",argc);

    if(argc<2) {
       printf("No argument specified\n");
       strcpy(dir,"Default argument");

    }
    else{
        printf("Command line directory was specified\n");
        ++argv;
        strcpy(dir,argv[0]);
    }

    strcat(dir,"_CAT");
    return dir;
}

int main(int argc, char *argv[]){

    char* dir;
    dir = handle_input(argc,argv);
    printf("This one messes it up!\n");
    printf("%s\n",dir);
    printf("DONE\n");
    return 0;
}
Yu Hao
  • 119,891
  • 44
  • 235
  • 294
Anthony Fouad
  • 53
  • 4

3 Answers3

0

In your handle_input function, you're returning a local array, which is a huge no-no; after the function ends, that array no longer exists, and what you return is just nonsense. This invokes undefined behavior later in your program. That printf call happens to overwrite the memory which previously belonged to the array. Without the printf call, that memory just happens to be intact, which is why it comes out intelligibly.

The right way to return an array is like this:

char* handle_input(int argc,char *argv[]){
    char* dir = malloc(200 * sizeof(char)); // sizeof unnecessary for char,
                                            // but it's a good habit to have
    printf("Number of arguments: %d\n",argc);

    if(argc<2) {
       printf("No argument specified\n");
       strcpy(dir,"Default argument");

    }
    else{
        printf("Command line directory was specified\n");
        ++argv;
        strcpy(dir,argv[0]); // This is unsafe! use strncpy instead
    }

    strcat(dir,"_CAT");
    return dir;
}

Using malloc ensures that the memory allocated will continue to exist after the end of the function, and that you will no longer be causing undefined behavior. Note that arrays are somewhat "special"; you can return primitive values (int, char, etc) and structs without having this sort of behavior occur.

Chris Hayes
  • 11,471
  • 4
  • 32
  • 47
  • Okay, Thanks! I was intending to return a pointer to an array... where did I go wrong? – Anthony Fouad Sep 25 '13 at 03:37
  • @ Chris thanks SOOO much :) One thing you forgot but I figured out... the malloc needs to be recast as a (char*) here. Thanks again! – Anthony Fouad Sep 25 '13 at 03:49
  • @AnthonyFouad If you're writing C casting malloc is unnecessary. If you're writing C++ you almost certainly shouldn't be using malloc (or printf, or raw C strings and the str* functions). – Nigel Harper Sep 25 '13 at 04:05
  • @Anthony: The reason you need to recast is probably because you forgot to `#include ` (which I should have mentioned in my answer). If you do that, casting is not necessary. – Chris Hayes Sep 25 '13 at 04:08
  • @AnthonyFouad writing that cast causes undefined behaviour , if it would not compile without it. You should avoid "fixing" warnings/errors by casting, unless you are 100% sure you understand what is happening and there is no other way. – M.M Aug 03 '15 at 03:42
0

You are returning a local char dir[200] in the function handle_input, after the function exits, it's undefined behavior to access it.

Use dynamic memory allocation instead, like:

char* handle_input(int argc,char *argv[]){
    char *dir = malloc(200);
    //do the processing
    return dir;
}

Remember to free it once it's out of use.

Yu Hao
  • 119,891
  • 44
  • 235
  • 294
0

You are returning a local vasriable

Either

  1. Use malloc (as you are in the C world with printf)
  2. Pass in a pointer to be filled up
Ed Heal
  • 59,252
  • 17
  • 87
  • 127