Returning char pointer from C function

Question

I need help to check if my code is correct. The code is too big to include entirely, so I will paste only the affected parts.

char *tmp;
tmp=Decode_URL(tmp_data);
sprintf(Data,"%s",tmp);
tmp=Decode_URL(tmp_backup1);
sprintf(DataB[0],"%s",tmp);
tmp=Decode_URL(tmp_backup2);
sprintf(DataB[1],"%s",tmp);
tmp=Decode_URL(tmp_backup3);
sprintf(DataB[2],"%s",tmp);
tmp=Decode_URL(tmp_backup4);
sprintf(DataB[3],"%s",tmp);
tmp=Decode_URL(tmp_backup5);
sprintf(DataB[4],"%s",tmp);

The Decode_URL function returns a char *.

So my question is, is it correct to always use tmp to receive the char * returned by the function? Or I should create more char *tmpx, one for each call to Decode_URL?

EDIT FOR MORE INFO:

char *Decode_URL(char *url){
      char *check;
      check=EnDeCrypt(some vars here);
      return check;
}

char *EnDeCrypt(const char *pszText, int iTextLen, const char *pszKey)
{
    char *cipher;
    int a, b, i=0, j=0, k;
    int ilen;
    int sbox[256];
    int key[256];

    ilen = strlen(pszKey);

    for (a=0; a < 256; a++)
    {
        key[a] = pszKey[a % ilen];
        sbox[a] = a;
    }

    for (a=0, b=0; a < 256; a++)
    {
        b = (b + sbox[a] + key[a]) % 256;
        swapints(sbox, a, b);
    }

    cipher = (char *)malloc(iTextLen);

    for (a=0; a < iTextLen; a++)
    {
        i = (i + 1) % 256;
        j = (j + sbox[i]) % 256;
        swapints(sbox, i, j);
        k = sbox[(sbox[i] + sbox[j]) % 256];
        cipher[a] = pszText[a] ^ k;
    }
    return cipher;
}

Thanks

its ok with one tmp!!! till the previous one is not used again. — akp
– akp, Commented Apr 26, 2013 at 12:05
depends on how that *char is created, stack wise, heap or returning const. — Grady Player
– Grady Player, Commented Apr 26, 2013 at 12:06
its probably ok until and unless the returned value is not the address of a local variable of the Decode_url() — Koushik Shetty
– Koushik Shetty, Commented Apr 26, 2013 at 12:06
It depend on what you return from Decode_URL. If you return a pointer to a local variable (like an array inside the function) then you suddenly have what is known as undefined behavior. — Some programmer dude
– Some programmer dude, Commented Apr 26, 2013 at 12:07
That will almost work. I say almost because you don't terminate the string you return, you don't even allocate memory for the string terminator. You also have a memory leak, you allocate memory for the returned string but you never free it. — Some programmer dude
– Some programmer dude, Commented Apr 26, 2013 at 12:17

unwind · Accepted Answer · 2013-04-26 12:06:06Z

5

This of course depends on where the memory comes from, that Decode_URL() is returning a pointer to.

If it's a static array, your usage is fine.

If it's dynamically allocated (by malloc() or any of its friends), then you're leaking memory.

answered Apr 26, 2013 at 12:06

unwind

402k64 gold badges492 silver badges620 bronze badges

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

4 Comments

Paul Over a year ago

That usage is undefined behaviour for a static array defined in the scope of Decode_URL.

Koushik Shetty Over a year ago

+1 for mentioning memory leak. also adding about returning local array address will complete the answer.

Koushik Shetty Over a year ago

@Paulpro how so? standard guarentees storage durationof static objects to be the same as the lifetime of execution of program?

Paul Over a year ago

@Koushik Teh standard guarantees static memory as long as that part of the stack is still in use. Once Decode_URL returns, it is not.

fotanus · Accepted Answer · 2013-04-26 12:06:00Z

2

It is correct. you could also go with

sprintf(DataB[4],"%s",Decode_URL(tmp_backup5))

answered Apr 26, 2013 at 12:06

fotanus

20.3k15 gold badges81 silver badges112 bronze badges

Comments

luser droog · Accepted Answer · 2013-04-26 12:05:51Z

0

As long as you don't need to do anything more with the data, sure, why not?

answered Apr 26, 2013 at 12:05

luser droog

19.6k3 gold badges58 silver badges107 bronze badges

Comments

Nathan Day · Accepted Answer · 2013-04-26 12:13:31Z

0

What we need to really know is what Decode_URL does to create the memory that the returned char pointer points to, is it malloc in which case you will need to release it, is it static in which case you need to be careful how you use the function and the data returned as the function will then not be reentrant nor thread-safe.

answered Apr 26, 2013 at 12:13

Nathan Day

6,0572 gold badges26 silver badges40 bronze badges

Comments

Community · Accepted Answer · 2020-06-20 09:12:55Z

0

It is correct since no other usage is being made with tmp, though you might want to consider

a macro for this:

#define SET_TEMP(BACKUP, TMP, DATA) \
        TMP=Decode_URL(BACKUP);     \
        sprintf(DATA,"%s",TMP)

and use it like this:

SET_TEMP(tmp_backup1, tmp, DataB[0]);
...

anyway, you are good to go, macros is just a better style of coding.

EDIT:

after seeing your added code, you do have a memory problem.

this is how your macro should look like:

#define SET_TEMP(BACKUP, TMP, DATA) \
        TMP=Decode_URL(BACKUP);     \
        sprintf(DATA,"%s",TMP);     \
        free(TMP)

and use it like this:

SET_TEMP(tmp_backup1, tmp, DataB[0]);
...

edited Jun 20, 2020 at 9:12

CommunityBot

11 silver badge

answered Apr 26, 2013 at 12:14

Itzik984

17k29 gold badges74 silver badges110 bronze badges

Collectives™ on Stack Overflow

Returning char pointer from C function

5 Answers 5

4 Comments

Comments

Comments

Comments

EDIT:

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

5 Answers 5

4 Comments

Comments

Comments

Comments

EDIT:

Comments

Your Answer

Sign up or log in

Post as a guest

Related