Jimmy Foo
Jimmy Foo

Reputation: 1

C char * function problems

I need help, I'm just learning C, have no idea what is wrong:

Here I call set_opts function :

char * tmploc ;
tmploc=set_opts("windir","\\temp.rte");
printf(tmploc);

( I know , that printf is not formated, just used it for testing purposes)

function looks like this :

char * set_opts(char * env,char * path){
    char * opt;
    opt=malloc(strlen(env)+strlen(path)+1);
    strcpy(opt,getenv(env));
    strcat(opt,path);
    return opt;
}

Everything is ok, but when I try to call it again :

char * tmploc2 ;
tmploc2=set_opts("windir","\\temp.rte");
printf(tmploc2);

...program just terminates

Please tell me what I'm doing wrong

Upvotes: 0

Views: 823

Answers (5)

Mike
Mike

Reputation: 49373

Be careful what you are doing with getenv(), because:

The getenv() function returns a pointer to the value in the environment, or NULL if there is no match.

So if you pass a name that does not correspond to an existing environment variable then you get NULL returned and that is going to kill your strcpy(opt,getenv(env));

I recomend:

  1. Check what malloc() returns and make sure that it is non-null.
  2. Check what getenv() returns and make sure that it is non-null.
  3. As you pointed out, use a format string in your printf's and compile with -Wall.
  4. Step through your code with a debugger to make sure it not terminating before you see the output.

Upvotes: 4

DWilches
DWilches

Reputation: 23015

Are you sure: getenv(env) will fit into "opt" ? I don't think so. And if it doesn't fit, then the strcpy could kill your program.

A correction: char * set_opts(char * env,char * path){ char * opt; char * value = getenv(env); opt=malloc(strlen(value)+strlen(path)+1); strcpy(opt,value); strcat(opt,path); return opt; }

This way, you're sure you have enough space.

Upvotes: 0

David Heffernan
David Heffernan

Reputation: 612884

You allocate the length of the string using env, but then populate it with getenv(env). If getenv(env) is longer than env then you have a good chance of a segfault. Did you mean to use strlen(getenv(env))?

You really ought to add some error checking to your code:

char *set_opts(char *env, char *path)
{
    char *opt;
    char *value;

    value = getenv(env);
    if (value == NULL)
      ... handle error
    opt = malloc(strlen(value)+strlen(path)+1);
    if (opt == NULL)
      ... handle error
    strcpy(opt,value);
    strcat(opt,path);
    return opt;
}

Upvotes: 6

Jens
Jens

Reputation: 72639

One possible reason: malloc returns NULL and you never check for it. Same for getenv(). And it must be

 malloc(strlen(getenv(env))+strlen(path)+1);

If the actual contents of getenv("windir") is longer than 6 characters, you write past the malloced buffer, which invokes undefined behavior.

Upvotes: 0

alf
alf

Reputation: 681

Try getting rid of getenv(env). Just put strcpy(opt,env);. getenv() is probably returning NULL.

Upvotes: 0

Related Questions