CODEWITHSUNDEEP
cpointerschar
user1042850
user1042850

Reputation: 125

Returning char pointer from C function

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

Upvotes: 0

Views: 7392

Answers (5)

Itzik984
Itzik984

Reputation: 16804

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]);
...

Upvotes: 0

Nathan Day
Nathan Day

Reputation: 6037

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.

Upvotes: 0

unwind
unwind

Reputation: 400079

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.

Upvotes: 5

fotanus
fotanus

Reputation: 20116

It is correct. you could also go with

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

Upvotes: 2

luser droog
luser droog

Reputation: 19514

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

Upvotes: 0

Related Questions