tags:

views:

207

answers:

10

I've tried everything Google-able, but can't seem to wrap my head around this. I'm trying to return a char and pass to another function with no luck. Do I need to do some sort of memory location?

char hostname[] = "www.XXXXX.com";
uint8 ipaddr[]  = {XXX,XXX,XXX,XXX};
char uri[]      = "/api/Login/";  
char key[] = API_KEY;  //3490324032-asdfasd-234234
const int port  = 80;

//function to combine uri and key
char combine(char key[], char uri[]) {
  int i = 0;
  int x = 0;
  char long_s[sizeof(uri) + sizeof(key)];

  while(uri[i]) {
      long_s[i] = uri[i];
      ++i;
  }
  while(key[x]) {
      long_s[i] = key[x];
      ++i;
      ++x;
  }
  long_s[i] = '\0';
  //Serial.print(long_s);
  return long_s;  //pointer?
} 

char lon = combine (key, uri);
char* newURI = &lon;

// A get request
GETrequest getSession(ipaddr, port, hostname, newURI);
A: 

You need to change combine to handle a char* as well.

//function to combine uri and key
void combine(char key[], char uri[], char long_s[]) {
  int i = 0;
  int x = 0;
  while(uri[i]) {
      long_s[i] = uri[i];
      ++i;
  }
  while(key[x]) {
      long_s[i] = key[x];
      ++i;
      ++x;
  }
  long_s[i] = '\0';
  //Serial.print(long_s);
}

char newURI = malloc(strlen(uri) + strlen(key) + 1);
combine(key, uri, newURI);

Then, you have to change the lon parameter to be a char*, too.

As pointed out by the comment, you actually have to do more than this, since long_s is a local variable.

You might have to change your function to take lon as a parameter and to intialize it outside of the function.

Again, this is quick code and you will have to tailor this to suit your needs (and do garbage collection).

palswim
This "return long_s;" would surely fail (local variable)
rubber boots
There's more to it than that - if a string (or char*) is being returned it has to point to something that will outlive the function call, so returning the address of a local won't do. The returned pointer must point to a dynamically allocated buffer or a static - both of which introduce issues of their own (lifetime management or sharing issues). An alternative is to have the output buffer passed in as an argument.
Michael Burr
Returning a pointer to an automatic variable is meaningless - the array `long_s` ceases to exist once the function returns.
caf
Your malloc is too short by one (for the 0-terminator).
Steve Jessop
That's what I get for posting quick code. Thanks Steve; fixed that.
palswim
+1  A: 

You say the function is returning a char, but yet you try to return a char* (the reference to the array). So you either need to change the function to be a char*, or you need to return an actual char. Also, you return a reference to memory that is no longer valid, you need to either malloc on the heap, or pass in the pointer to be filled with the function call.

Edit: Further clarification

All of these are pointers to arrays of type uint8 or char.

char hostname[] = "www.XXXXX.com";
uint8 ipaddr[]  = {XXX,XXX,XXX,XXX};
char uri[]      = "/api/Login/";  
char key[] = API_KEY;  //3490324032-asdfasd-234234

This is regular data.

const int port  = 80;

//function to combine uri and key
char combine(char key[], char uri[]) {
  int i = 0;
  int x = 0;

This next line declares a pointer to an array of chars that has 8 elements. This probably isn't what you want. The sizeof macro is evaluating to 4 because that is the size of a pointer (compile and run the program below). Also you need to change this to char * long_s = malloc(sizeof(uri) + sizeof(key)); Dont forget to free it later.

  char long_s[sizeof(uri) + sizeof(key)];
  while(uri[i]) {
      long_s[i] = uri[i];
      ++i;
  }
  while(key[x]) {
      long_s[i] = key[x];
      ++i;
      ++x;
  }
  long_s[i] = '\0';
  //Serial.print(long_s);

This returns the pointer to the array that you just filled, not an actual char.

  return long_s;  //pointer?
} 

These next lines should just be char * newURI = combine(key, uri);

char lon = combine (key, uri);
char* newURI = &lon;

// A get request
GETrequest getSession(ipaddr, port, hostname, newURI);

Edit: Try this out:

#include <stdio.h>

void test(char k[])
{
    printf("%d\n", sizeof(k));
}

int main ()
{
    char* key[100];
    test(key);
    printf("%d\n", sizeof(key));
    return 0;
}
Seamus
+1  A: 

You are trying to return a character array, not a character. Define the function as

char *combine(char key[], char uri[])

and

char *newURI = combine(key, uri);

Edit: as mentioned in several other comments, the function's body is also a problem because you return a string that only makes sense within the scope of the function. You could do the following instead:

void combine(char key[], char uri[], char *dest)
{
... and place the result in dest
}

and

char *newURI = (char*)malloc(strlen(key) + strlen(uri) + 1);
combine(key, uri, newURI);
... do whatever
free(combine);
Jonathan
This is likely to steer the OP down the wrong track though - the contents of the function also need to change. Even though it would *compile* with these changes, and may even appear to work, it still wouldn't be correct.
caf
Yes, you're right. I'll edit accordingly.
Jonathan
A: 

If you are planning to return a pointer to char, your function should be declared as

char *combine (char key[], char uri[])
{
    //...etc...
    return long_s;  //pointer?
} 
Brian Hooper
+6  A: 

The combine function above purports to return a char, but in fact the return long_s is returning a char* as the comment after this line suspects. However, changing the function to return a char* will not really get you anywhere as returning the address of a string (char array) on the stack is a dangerous game - once the function has returned you can't rely on this pointer being valid. It's better to either have a third parameter to combine, char result[] into which the result is placed, or to return a malloc'd char array and ensure that you free() this at some point.

Will A
Since this is tagged `c`, it would probably be better to use the terms `malloc` and `free` instead of `new` and `delete[]`. Otherwise, +1
Nick Presta
@Nick - thanks, good point - amended to reflect this.
Will A
+2  A: 

You're not trying to return a char, you're trying to return a string.

A char in C is a character (or a byte). A string is a sequence of characters. Yes, you need some sort of memory location, either allocated in your combine function with malloc, or supplied by the caller as an extra parameter. Or more likely two extra parameters, a pointer and a length, to help ensure there's enough space to write the concatenated strings.

Everying in C is passed (and returned) by value[*]. So when you return a char, you're just returning one of the possible values a char can take. There's no way you can return a char from the beginning of a string, take a pointer to the return value, and get the rest of the string. By analogy if I tell you that I'm thinking of the letter "a", it doesn't make sense to ask whether I mean the "a" in "ask" or the "a" in "mean". I just mean a letter "a": no further context. You need pointers if you want to indicate which string that letter "a" belongs to.

Also, sizeof doesn't measure the length of a string. It gives you the size of the type or object which is its operand. Due to an unfortunate peculiarity of the syntax of C, your parameters key and uri are of type char* - they're pointers, so their size is the size of a pointer, not the length of the array that it points to (the first element of). So, your long_s buffer is only 8/16 bytes big (on a typical 32/64 bit architecture), and will be overflowed. This results in undefined behaviour, meaning that it might crash, it might appear to work, or it might allow an attacker to gain control of your machine.

Finally, there are standard functions strlen, strcat and strncat which you could look at to see how string handling works in C. Here's one way to do it:

char *newURI = malloc(strlen(uri) + strlen(key) + 1);
if (newURL == 0) { /* indicate failure, somehow */ }
strcpy(newURI, uri);
strcat(newURI, key);
GETrequest session = getSession(ipaddr, port, hostname, newURI);
free(newURI); // avoids a memory leak
// do something with the session...

[*] even pointers are passed by value.

Steve Jessop
+1  A: 

The function doesn't return a char, but a string. Change the the function signature into

char* combine(char key[], char uri[]) {

(make it return char* instead of char) and call it like this:

char* newURI = combine(key, uri);
snakile
A: 

First problem: you're clearly trying to return a string from your combine function, not a character. So why are you declaring it as returning char? It needs to return the type of strings, i.e. char*.

Second problem: functions can't really take array parameters in C. When you declare combine as taking two array parameters, it's exactly equivalent as declaring it as taking two pointer parameters. It's better to explicitly declare the pointers as parameters, because then you would have a chance to avoid the next problem. So far we have:

char* combine(char *key, char *uri) {

Third problem: since key and uri are not arrays but pointers, sizeof(key) and sizeof(uri) give you the size of the pointers, which is not useful here. What you need instead, in this particular is the length of the strings. And you need to add 1 to allow for the '0' at the end of the combined string.

Fourth problem: if you allocate long_s as a local array variable, the memory allocated to it is reclaimed at the end of the function. So you can't return it. In order to get a chunk of memory that survives the end of the function, you need to use dynamic memory allocation, using malloc:

char *long_s = malloc(strlen(key) + strlen(uri) + 1);
if (long_s == NULL) { ... /*out of memory*/ }

Once these problems are fixed, you can directly do char *newURI = combine(key, uri);. Don't forget to free(newURI) when you've finished using it.

If this is a real program rather than a learning exercise, learn to use the string library functions. In particular, your combine function is more easily written as strcpy followed by strcat.

Gilles
A: 

Your function return value is char - that means that it returns a single char (which can only represent one character). A string is represented by an array of chars, which is usually referred to by a pointer (char *).

So you need to return a char * - but this leads to another problem. The pointer must be pointing at an array that outlives the function call. Your array long_s will be destroyed as soon as the function returns, so this is not suitable. The usual alternative is to allow the caller to supply a buffer of sufficient size (note also that your loops can be replaced with the strcpy() and strcat() functions):

char *combine(char result[], size_t result_sz, const char key[], const char uri[])
{
  /* Test to see if there is sufficient space */

  if (strlen(uri) + strlen(key) + 1 > result_sz)
  {
    return NULL;
  }

  strcpy(result, uri);
  strcat(result, key);

  return result;
} 

Then your caller must use:

char lon[1024];
char *newURI = combine (lon, sizeof lon, key, uri);

if (newURI == NULL)
{
    /* Error - URL too long */
}
else
{
    GETrequest getSession(ipaddr, port, hostname, newURI);
}
caf
+1  A: 

You have a couple of problems:

  1. you're returning a pointer to a local variable. While you might be able to accidentally print reasonable looking output from that result, there's a bug here in that you're accessing data that's no longer valid. The local variable the result is pointing to is no longer 'alive' once the function returns

  2. you're sizing the result buffer incorrectly. The uri and key parameters are really pointers (see Sizeof array passed as parameter), so

    char long_s[sizeof(uri) + sizeof(key)];
    

    actually gives you an array of characters large enough to hold 2 pointers, which isn't necessarily large enough to hold your strings.

You need to decide how you want to manage the combined string. There are a couple options:

  1. Have your callers pass in a buffer to use (along with the size so you can prevent overrun errors)

  2. Have the function allocate the buffer and require the caller to free it when they no longer need it.

An example using option 2:

//function to combine uri and key
char* combine(char const* key, char const* uri) {
    int len = strlen(key) + strlen(uri) + 1; /* don't the '\0' terminator */

    char* pResult = malloc(len);

    if (pResult) {
        /* we got a buffer */
        strcpy( pResult, uri);
        strcat( pResult, key);
    }

    return pResult; /* will return NULL on failure */
                    /* caller is responsible for calling free() */
} 
Michael Burr