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

char *chktype(char *Buffer, int Size)
{
   char *strng = "Content-Type: ";
   int sz;
   char *found = strstr (Buffer, strng);
   char *found1 = strstr(found, "\r\n");
   sz=strlen(found)-strlen(found1);
   char type[sz];
   strncpy(type, found1, sz-1);

   return(type);
}

void main(){

   char *buffer = "HTTP/1.1 200 OK\r\nDate: Tue, 25 Jun 2013 16:27:16
   GMT\r\nExpires: -1\r\nCache-Control: private,
   max-age=0\r\nContent-Type: text/html; 
   charset=UTF-8\r\nContent-Encoding: gzip\r\nServer: 
   gws\r\nX-XSS-Protection: 1; mode=block\r\nX-Frame-Options:
   SAMEORIGIN\r\nTransfer-Encoding: chunked\r\n\r\n";

   char *extension = chktype (buffer, sizeof(buffer));
   printf("%s\r\n", extension);
}

This yields:

warning: function returns address of local variable [enabled by 
default]

...and i can't figure out what is wrong here. When I run it, I expect the output to be text/html; charset=UTF-8 but its gibberish.

What does the warning mean exactly?

Gewure
  • 1,208
  • 18
  • 31
aDi Adam
  • 93
  • 1
  • 1
  • 4
  • 1
    You shouldn't return local arrays from a function. One way as shown is to declare them static which will make them persist for the entire duration of the execution. The other is to allocate them using malloc. – Nobilis Jun 26 '13 at 05:47
  • 3
    What do you even expect? Your code is a terrible mess, no one can read it, I'm sure even you can't. ***Format it.*** –  Jun 26 '13 at 05:48
  • Also, rolled back to v1. No-one wants those `??`s. We have enuf of'em already. –  Jun 26 '13 at 05:55

4 Answers4

16

The chktype function allocates memory for an automatic variable on the stack, and then returns the address of this variable (i.e., a pointer to this variable).

The problem is that variables allocated on the stack are automatically destroyed whenever they go out of scope (i.e., control passes outside of the curly braces that define the function).

This means that you're essentially returning a pointer to an invalid memory location, which is bad news. In C-speak, it's undefined behavior. In practical speak, it results in bad output or perhaps even a crash.

char *chktype(char *Buffer, int Size)
{
    // This pointer variable is allocated on the stack, but that's okay because
    // it's a pointer to a string literal, which are always constant.
    // (Technically, you should add the "const" qualifier to the declaration.)
    const char *strng = "Content-Type: ";

    int sz;
    char *found = strstr (Buffer, strng);
    char *found1 = strstr(found, "\r\n");
    sz=strlen(found)-strlen(found1);

    // Like all the above variables, the one is also allocated on the stack.
    // But it's the source of your problem here, because it's the one that
    // you are returning at the end of the function.
    // Problem is, it goes away at the end of the function!
    char type[sz];
    strncpy(type, found1, sz-1);
    return(type);
}

The correct way to return a char* from a function is to allocate new memory from the heap using the malloc (or calloc) function. That means that the caller of the function is going to be responsible for freeing the memory used by the returned value, otherwise your program will leak memory.
(Always put this requirement into the documentation for your function! Even if "documentation" means a comment above the declaration.)

For example, change your code to look like this:

char *chktype(char *Buffer, int Size)
{
    // This pointer variable is allocated on the stack, but that's okay because
    // it's a pointer to a string literal, which are always constant.
    // (Technically, you should add the "const" qualifier to the declaration.)
    const char *strng = "Content-Type: ";

    int sz;
    char *found = strstr (Buffer, strng);
    char *found1 = strstr(found, "\r\n");
    sz=strlen(found)-strlen(found1);

    char *type = malloc(sz);  // allocate memory from the heap
    strncpy(type, found1, sz-1);
    return(type);
}

Now, in the caller of the chktype function, you must make sure that you call free whenever you are finished with its return value:

char *type = chktype(...);
// do something
free(type);

Note that robust code should test the result of malloc for a null pointer to make sure that it did not fail to allocate the requested memory. If so, you need to handle the error somehow. For clarity, that isn't shown above.

Cody Gray - on strike
  • 239,200
  • 50
  • 490
  • 574
  • thank u. i now see what iwa s dooing wrong. i did that part and i am getting the one content header as a string. this is the output. 'Content-Type: text/html; charset=UTF-8' now how do i remove the first 17 characters from the string?? i want to remove the 'Content-Type: ' from the string or i can do the same by copying the string from the 18th character and onwad. how can i acomplish that?? – aDi Adam Jun 26 '13 at 15:44
  • @aDiAdam Yes, use the `strncpy` function like you did before. Start copying at the 18th character. You'll get a new string that does not have the first 17 characters in the original. You can't remove them in-place in C. [String handling is a pain in the butt in low-level languages](http://www.joelonsoftware.com/articles/fog0000000319.html), that's why people use C++ that gives you a built-in `string` class. – Cody Gray - on strike Jun 28 '13 at 00:00
  • isn't there any other way of somehow keeping the malloc-free responsibility in the scope of the original function? Encapsulation etc.. ? – Gewure Feb 22 '17 at 09:10
8

Quick/Hacky answer(?):

Make

char type[sz];

into

static char type[sz];

Long answer: The error is pretty clear, you are returning the address of a variable that is going to be destroyed soon as the function returns. There are a couple of ways to get around this.

One easy way is to make type static, this would fix things, by making the type variable have a lifetime of the program, but this will mean that you cannot call it twice in a row, you need to print or copy the result before calling again.

The other way, is to allocate memory for a char array within your function and hope that you remember to free it once you are done with it. If you don't you will have a memory leak. This does not suffer from the above disadvantage.

Karthik T
  • 31,456
  • 5
  • 68
  • 87
2

When you declare type as char type[sz], that's giving you a local variable. The lifetime of that memory will end when the function returns. Instead, you need to dynamically allocate memory, for example, using malloc.

char *type = (char *) malloc (sz * sizeof (char));
Keith
  • 518
  • 1
  • 6
  • 10
  • 3
    I. e. [***Pretty please don't do it!***](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc/605858#605858) –  Jun 26 '13 at 05:52
  • Also, "giving you a local variable" is not a problem. KarthikT's solution also uses a local variable yet it's correct. The problem is that it has automatic storage duration. –  Jun 26 '13 at 05:54
0

You return type, which points to an array which had been allocated on the stack and is invalid after the function chktype() has returned.

You might like to allocate the result on the heap, like this:

char * chktype(const char * buffer, int size)  
{
  char * strng = "Content-Type: ";
  char * found = strstr (buffer, strng);
  char * found1 = strstr(found, "\r\n");
  size_t sz = strlen(found) - strlen(found1);
  char * type = calloc(sz, sizeof(*type));
  if (type)
  {
    strncpy(type, found1, sz - 1);
  }

  return type;
}

However, there is the need to free() the result after not needed anymore.

alk
  • 69,737
  • 10
  • 105
  • 255
  • okay i used malloc and it works, type now contains "Content-Type: text/html; charset=UTF-8" i want to remove the say first 17 charachters from this string, how do i go about that?? – aDi Adam Jun 26 '13 at 16:00
  • @aDiAdam: What do to is the string is less then 17 characters long? – alk Jun 26 '13 at 16:04
  • @aDiAdam: `{char * type = chkType(...); size_t n = MIN(17, strlen(type); memmove(type, type + n * sizeof(*type), n * sizeof(*type));}` – alk Jun 26 '13 at 16:06
  • no what i want is a string "text/html; charset=UTF-8" so i can either copy 18th character onward from he string to another string. or i smhow delete the "Content-Type: " from the same string. and i am sorry i am a little rusty with the string library functions. can u please elaborate a little on the memove() part u have written. i will be very grateful – aDi Adam Jun 26 '13 at 16:19
  • I missed a `)` after `strlen(type)`. For details please see `man memmove`. – alk Jun 26 '13 at 16:31
  • /tmp/ccciGebX.o: In function `chktype': test.c:(.text+0x9b): undefined reference to `MIN' collect2: error: ld returned 1 exit status – aDi Adam Jun 26 '13 at 17:23
  • i changed the program to `strncpy(type, found, sz); size_t n = MIN(17, strlen(type)); memmove(type, type + n * sizeof(*type), n * sizeof(*type)); if (strncmp("Content-Type: text/html" , type, 23) == 0) { return("type"); }` but it is generating an error as mentioned in the previous comment – aDi Adam Jun 26 '13 at 17:25