I have function:
char *zap(char *ar) {
char pie[100] = "INSERT INTO test (nazwa, liczba) VALUES ('nowy wpis', '";
char dru[] = "' )";
strcat(pie, ar);
strcat(pie, dru);
return pie;
}
and in main there is:
printf("%s", zap( argv[1] ) );
When compiling I get the warning:
test.c: In function ‘zap’:
test.c:17: warning: function returns address of local variable
How should I return char* propertly?
Your best bet probably is not to return it at all - instead, pass the buffer you want to populate into the function as a parameter.
void zap(char * pie, const char *ar) {
strcpy( pie, "INSERT INTO test (nazwa, liczba) VALUES ('nowy wpis', '");
char dru[] = "' )";
strcat(pie, ar);
strcat(pie, dru);
}
Then call it like this:
char pie[100];
zap( pie, "foo" );
To bullet proof this function, you also need to pass in the length for the buffer, and then check against this every time you are about to add a new query element.
The posted solutions all work, but just to answer your question as to why you get a warning:
When you declare pie as buffer in the function, you are not allocating heap memory, the variable is being created in the stack. That memory content is only guaranteed within the scope of that function. Once you leave the function (after the return) that memory can be reused for anything and you could find that memory address you are pointing at overwritten at any time. Thus, you are being warned that you are returning a pointer to memory that is not guaranteed to stick around.
If you want to allocate persistent memory in a c function that you can reference outside that function, you need to use malloc (or other flavors of heap memory allocation functions). That will allocate the memory for that variable on the heap and it will be persistent until the memory is freed using the free function. If you aren't clear on stack vs. heap memory, you may want to google up on it, it will make your C experience a lot smoother.
Allocate the memory for pie with malloc
#include <assert.h>
#include <stdio.h>
/**
* Returns the buffer, just for convenience.
*/
char *generateSQL(char *buf, size_t bufsize, const char *ar) {
int n;
n = snprintf(buf, bufsize, "INSERT INTO test (nazwa, liczba) VALUES ('nowy wpis', '%s')", ar);
/* FIXME: Properly escape the argument, just in case it contains an apostrophe. */
assert(0 <= n && (unsigned) n < bufsize);
return buf;
}
int main(int argc, char **argv)
{
char buffer[4096];
assert(1 < argc);
printf("%s\n", generateSQL(buffer, sizeof(buffer), argv[1]));
return 0;
}
char pie[100];
void zap(char* pie, char *ar) {
char pies[100] = "INSERT INTO test (nazwa, liczba) VALUES ('nowy wpis', '";
char dru[] = "' )";
strcpy(pie, pies);
strcat(pie, ar);
strcat(pie, dru);
}
zap(pie, argv[1]);
printf("%s", pie );
I would strongly suggest changing this function, let the user pass a buffer and a length as well and use that buffer instead. Alternatively you can allocate a new instance of the return value i.e. with malloc but make sure to leave a comment to the user to free it again.
declare your char array static
static char pie[100] = "INSERT INTO test (nazwa, liczba) VALUES ('nowy wpis', '";
Slightly different approach:
void zap(char **stmt, char *argument, size_t *stmtBufLen)
{
char *fmt="INSERT INTO test(nazwa, liczba) VALUES ('nowy wpis', '%s')";
/**
* Is our current buffer size (stmtBufLen) big enough to hold the result string?
*/
size_t newStmtLen = strlen(fmt) + strlen(argument) - 2;
if (*stmtBufLen < newStmtLen)
{
/**
* No. Extend the buffer to accomodate the new statement length.
*/
char *tmp = realloc(*stmt, newStmtLen + 1);
if (tmp)
{
*stmt = tmp;
*stmtLen = newStmtLen+1;
}
else
{
/**
* For now, just write an error message to stderr; the statement
* buffer and statement length are left unchanged.
*/
fprintf(stderr, "realloc failed; stmt was not modified\n");
return;
}
}
/**
* Write statement with argument to buffer.
*/
sprintf(*stmt, fmt, argument);
}
int main(void)
{
char *stmtBuffer = NULL;
size_t stmtBufferLen = 0;
...
zap(&stmtBuffer, "foo", &stmtBufferLen);
...
zap(&stmtBuffer, "blurga", &stmtBufferLen);
...
zap(&stmtBuffer, "AReallyLongArgumentName", &stmtBufferLen);
...
zap(&stmtBuffer, "AnEvenLongerRidiculouslyLongArgumentName", &stmtBufferLen);
...
free(stmtBuffer);
return 0;
}
This version uses dynamic memory allocation to resize the buffer as needed, starting with a NULL buffer pointer (realloc(NULL, size) == malloc(size)). This way you don't have to worry about starting out with a buffer that's "big enough". The only drawback is you need to remember to deallocate the buffer when you're done with it (I don't normally like splitting memory management duties between caller and callee like this; if I thought about it for more than 10 minutes, I'd come up with something better).
The way I do such manipulations is making local buffer a static thread-specific variable:
const int max_pie_cnt = 100;
const char *zap(char *ar) {
static __declspec(thread) char pie[max_pie_cnt]; // use TLS to store buffer
strcpy(pie, "INSERT INTO test (nazwa, liczba) VALUES ('nowy wpis', '");
char dru[] = "' )";
strcat(pie, ar);
strcat(pie, dru);
return pie;
}
I'm very curious about experts' comments.
Btw, let's forget about buffer overflow problem for a moment.
