Compiler Warning

[srikar2097]

I have written this small fn. to strip down whitespaces ("\n") a given
string (only leading and trailing).
The program itself might not be foolproof but that is not my concern
right now. When I compile this
I get a warning :--

"string_utils.c:96: warning: function returns address of local
variable.

I have declared the fn. as fn. pointer and this fn. returns the
address of a string array. Why does
the compiler give out this warning? Please help.

Thanks,
Srikar


PROGRAM:
--------------

char *xstrip(char *); //declaration.

char *xstrip(char *c)
{
int i=0;
char tmp_str[xstrlen(&c[0])];

while(*c != '\0')
{
if(*c==32) // if space.
{
if((*(c+1)!=32 && *(c-1)!=32))
;
else
{
c++;
continue;
}
}
tmp_str=*c;
i++;
c++;
}
//Adding null char to indicate end of string. Else printf would
not know where str ends.
tmp_str='\0';

return &tmp_str[0];

}

 

[Bartc]

I have written this small fn. to strip down whitespaces ("\n") a given
string (only leading and trailing).
The program itself might not be foolproof but that is not my concern
right now. When I compile this
I get a warning :--

"string_utils.c:96: warning: function returns address of local
variable.

I have declared the fn. as fn. pointer and this fn. returns the
address of a string array. Why does
the compiler give out this warning? Please help.

char *xstrip(char *c)
{
int i=0;
char tmp_str[xstrlen(&c[0])]; ....
return &tmp_str[0];
}

The compiler is spot-on: you're returning the address of a local variable.

You can't do that (or at least, it's a very bad idea).

The tmp_str variable will cease to exist after returning, the address will
be meaningless.

You might try static in front it, although it looks like you want a dynamic
length for the array (not possible for static). You need a different
solution.

 

[vippstar]

I have written this small fn. to strip down whitespaces ("\n") a given
string (only leading and trailing).
The program itself might not be foolproof but that is not my concern
right now. When I compile this
I get a warning :--

"string_utils.c:96: warning: function returns address of local
variable.

I have declared the fn. as fn. pointer and this fn. returns the
address of a string array. Why does
the compiler give out this warning? Please help.

Thanks,
Srikar

PROGRAM:

Gee that's a helpful comment.

char *xstrip(char *c)
{
int i=0;
char tmp_str[xstrlen(&c[0])];

What is xstrlen? It's not provided. Does it expand to something
constant or not? Why &c[0] and not c? If it calculates the length of
the string c, then what will you do for empty strings? You can't have
0 length arrays!

while(*c != '\0')
{
if(*c==32) // if space.

32 is just a value. If you want to see whether *c is a space or not,
you'll have to use: ' ', like this:

if(*c == ' ')

If you want to know whether *c belongs in the space class, you'll have
to use isspace (<ctype.h>)

if(isspace((unsigned char)*c))

{
if((*(c+1)!=32 && *(c-1)!=32))

Ditto. c - 1 can invoke undefined behavior, unless you plan on
invoking the function as:
char foo[N];
/* ... */
foo[0] = 0;
xstrip(&foo[1]);

Or similar.

;
else
{
c++;
continue;
}
}
tmp_str=*c;
i++;
c++;
}
//Adding null char to indicate end of string. Else printf would
not know where str ends.
tmp_str='\0';

Assuming xstrlen served the purpose of an strlen, in case the string
doesn't contain any whitespace, you're writing past the end of
tmp_str. (it's easy to see this by taking c == "", nevermind there
can't be 0 sized arrays)

return &tmp_str[0];

tmp_str is a local object, you can't return its address.

 

[s0suk3]

I have written this small fn. to strip down whitespaces ("\n") a given
string (only leading and trailing).
The program itself might not be foolproof but that is not my concern
right now. When I compile this
I get a warning :--

"string_utils.c:96: warning: function returns address of local
variable.

I have declared the fn. as fn. pointer and this fn. returns the
address of a string array. Why does
the compiler give out this warning? Please help.

Try:

#include <stdlib.h>
#include <string.h>
#include <ctype.h>

// Return value must be manually deallocated

char* xstrip(const char* str)
{
while (isspace((unsigned char) *str) && *str != '\0')
++str;

const char* end = str + strlen(str);
while ((isspace((unsigned char) *end) || *end == '\0') && end >
str)
--end;

char* buf = (char*) calloc(1, end - str + 2);
if (!buf)
return NULL;

strncpy(buf, str, end - str + 1);
return buf;
}

Or, if you want to modify the string in-place:

void xstrip(char* str)
{
char* start = str;

while (isspace((unsigned char) *start) && *start != '\0')
++start;

char* end = start + strlen(start);
while ((isspace((unsigned char) *end) || *end == '\0') && end >
start)
--end;

memmove((void*) str, (const void*) start, end - start + 1);
str[end - start + 1] = '\0';
}

Sebastian

 

[srikar2097]

Thanks guys, as both Bart and Vippstar have mentioned - "Since tmp_str
is a local variable. It cannot exist outside that fn. So what I have
done is not valid."

But I need to access this (modified) array outside this fn. How should
I go about it?
This is a string array and I expect it to be very big in some cases.
So I don't want to copy the entire array outside this fn. Just a
pointer??

After some thought::- Since after the fn. "xstrip()" is executed all
the space allotted to the vars in this fn. is out of scope to the ones
outside it. I get this is the problem. So what is the way?

Thanks...

PS: "xstrlen()" is nothing but a spoof (my) implementation of "strlen
()".

I have written this small fn. to strip down whitespaces ("\n") a given
string (only leading and trailing).
The program itself might not be foolproof but that is not my concern
right now. When I compile this
I get a warning :--

"string_utils.c:96: warning: function returns address of local
variable.

I have declared the fn. as fn. pointer and this fn. returns the
address of a string array. Why does
the compiler give out this warning? Please help.


char *xstrip(char *); //declaration.

Gee that's a helpful comment.

char *xstrip(char *c)
{
    int i=0;
    char tmp_str[xstrlen(&c[0])];

What is xstrlen? It's not provided. Does it expand to something
constant or not? Why &c[0] and not c? If it calculates the length of
the string c, then what will you do for empty strings? You can't have
0 length arrays!

    while(*c != '\0')
    {
        if(*c==32) // if space.

32 is just a value. If you want to see whether *c is a space or not,
you'll have to use: ' ', like this:

if(*c == ' ')

If you want to know whether *c belongs in the space class, you'll have
to use isspace (<ctype.h>)

if(isspace((unsigned char)*c))

        {
            if((*(c+1)!=32 && *(c-1)!=32))

Ditto. c - 1 can invoke undefined behavior, unless you plan on
invoking the function as:
char foo[N];
/* ... */
foo[0] = 0;
xstrip(&foo[1]);

Or similar.

                ;
            else
            {
                c++;
                continue;
            }
        }
        tmp_str=*c;
        i++;
        c++;
    }
    //Adding null char to indicate end of string. Else printf would
not know where str ends.
    tmp_str='\0';

Assuming xstrlen served the purpose of an strlen, in case the string
doesn't contain any whitespace, you're writing past the end of
tmp_str. (it's easy to see this by taking c == "", nevermind there
can't be 0 sized arrays)

return &tmp_str[0];

tmp_str is a local object, you can't return its address.

}

 

[Nick Keighley]

don't top-post. That is don't post your reply before the text you
are replying to. I have re-arranged your post.

[/QUOTE]

perhaps it should be...

When I compile this I get a warning :--
"string_utils.c:96: warning: function returns address of local
variable.
I have declared the fn. as fn. pointer and this fn. returns the
address of a string array. Why does
the compiler give out this warning? Please help.
Thanks,
Srikar
PROGRAM:
--------------
char *xstrip(char *); //declaration.
char *xstrip(char *c)
{
int i=0;
char tmp_str[xstrlen(&c[0])];
while(*c != '\0')
{
if(*c==32) // if space.

32 is just a value. If you want to see whether *c is a space or not,
you'll have to use: ' ', like this:

if(*c == ' ')

If you want to know whether *c belongs in the space class, you'll have
to use isspace (<ctype.h>)

that "space class" includes other whitespace characters such as <tab>
\t

note this is one of the (few) cases where a cast is a good idea

{
if((*(c+1)!=32 && *(c-1)!=32))
;
else
{
c++;
continue;
}
}
tmp_str=*c;
i++;
c++;
}
//Adding null char to indicate end of string. Else printf would
not know where str ends.

beware // comments can screw up when posted to a news group.
/* comments tend to be safer

tmp_str='\0';

return &tmp_str[0];

tmp_str is a local object, you can't return its address.

Thanks guys, as both Bart and Vippstar have mentioned - "Since tmp_str
is a local variable. It cannot exist outside that fn. So what I have
done is not valid."
yep


But I need to access this (modified) array outside this fn. How should
I go about it?
This is a string array and I expect it to be very big in some cases.
So I don't want to copy the entire array outside this fn. Just a
pointer??

After some thought::- Since after the fn. "xstrip()" is executed all
the space allotted to the vars in this fn. is out of scope to the ones
outside it. I get this is the problem. So what is the way?

there are several cannonical ways to do this

1. modify the string you passed in. Don't use a temporary. Manipulate
c
directly. This won't work on a string literal (you aren't allowed to
modify them)
xstrip ("this string is mad, bad and dangerous to gnaw");

2. pass in a result string
char *xstrip (char *result, const char *to_be_stripped);

3. use malloc(). This has the problem that the caller must call
free()

4. use a decent string library. C is a bit weak out-of-the-box
in its string handling

5. use a static as a temporary. Can overflow the temp and your
function
is not re-entrant

6. use a global (file scope) variable. All the bad stuff of 5 plus
the extra problems of globals

those suggestions are roughly in order of preference

<snip>

--
Nick Keighley

A good designer must rely on experience, on precise, logic thinking;
and on pedantic exactness. No magic will do.
(Wirth)

 

[Keith Thompson]

I have written this small fn. to strip down whitespaces ("\n") a given
string (only leading and trailing).

I'm sure that "fn." means "function", but please take the time to
spell it out.

'\n' is the new-line character. There are several other characters
that are considered whitespace, including the space ' ' and tab '\t'
characters. The isspace() function is your best bet here.

The program itself might not be foolproof but that is not my concern
right now. When I compile this
I get a warning :--

"string_utils.c:96: warning: function returns address of local
variable.

Already answered, but see also the comp.lang.c FAQ,

I have declared the fn. as fn. pointer and this fn. returns the
address of a string array. Why does
the compiler give out this warning? Please help.

No, you haven't declared the function as a function pointer. A
"function pointer" is a pointer to a function; you haven't used any
function pointers (at least not explicitly).

The term "string array" is unclear. An array is a particular kind of
data type; you can have an array object, for example. A "string" is a
data *format*, not a data type; it's defined as "a contiguous sequence
of characters terminated by and including the first null character"
(C99 7.1.1p1). An array can contain a string.

PROGRAM:
--------------

char *xstrip(char *); //declaration.

char *xstrip(char *c)
{

This isn't wrong, but the separate declaration isn't really necessary
in this case; the "char *xstrip (char *c)" that's part of the function
definition also serves as a declaration. Also, you can include the
parameter name in a standalone declaration if you like; I generally
prefer to do so just because it's clearer.

Separating a declaration (prototype) from a definition (the body of
the function) is very useful when the function needs to be called from
another source file, or when you want to have calls preceding the
definition within a source file. In this particular case, I wouldn't
bother.

int i=0;
char tmp_str[xstrlen(&c[0])];

while(*c != '\0')
{
if(*c==32) // if space.
{
if((*(c+1)!=32 && *(c-1)!=32))
;

As somebody else mentioned, using the value 32 is a bad idea. 32
happens to be the numeric representation for the ' ' character in
ASCII, and it's very likely that every system you ever encounter will
use an ASCII-based character encoding, but (a) it's not guaranteed by
the language (EBCDIC has ' ' == 64), and (b) it's much clearer to use
a character constant ' '.

[snip]

return &tmp_str[0];

The above is incorrect, as has already been noted. But if it were
correct (say, if tmp_str were declared static), it would be simpler to
write:

return tmp_str;

Read section 6 of the comp.lang.c FAQ to understand why these are
equivalent.

 

[Barry Schwarz]

Try:

#include <stdlib.h>
#include <string.h>
#include <ctype.h>

// Return value must be manually deallocated

char* xstrip(const char* str)
{
while (isspace((unsigned char) *str) && *str != '\0')

If isspace returns true, can the second conditional expression ever be
false? If isspace returns false, the second expression won't be
evaluated. Why is it there?

++str;

const char* end = str + strlen(str);

Unless you have c99, declarations must precede statements.

while ((isspace((unsigned char) *end) || *end == '\0') && end >
str)
--end;

char* buf = (char*) calloc(1, end - str + 2);

Don't cast the return from calloc (or any standard *alloc).

You are paying to high a price for the single '/0' that survives the
call to strncpy below.

if (!buf)
return NULL;

strncpy(buf, str, end - str + 1);
return buf;
}

Or, if you want to modify the string in-place:

void xstrip(char* str)
{
char* start = str;

while (isspace((unsigned char) *start) && *start != '\0')

The second comparison is still pointless.

++start;

char* end = start + strlen(start);
while ((isspace((unsigned char) *end) || *end == '\0') && end >
start)
--end;

memmove((void*) str, (const void*) start, end - start + 1);

The casts serve no purpose since there is a prototype in scope
(courtesy of string.h)

str[end - start + 1] = '\0';
}

Sebastian

 

[s0suk3]

If isspace returns true, can the second conditional expression ever be
false?  If isspace returns false, the second expression won't be
evaluated.  Why is it there?

Explicitness, I guess.

Unless you have c99, declarations must precede statements.



Don't cast the return from calloc (or any standard *alloc).

That's got to be the most repeated line in this group

You are paying to high a price for the single '/0' that survives the
call to strncpy below.

Just because of setting all bits to zero? You know what they say about
premature optimization...

Sebastian

 

[Nick Keighley]

I have written this small fn. to strip down whitespaces ("\n") a given
string (only leading and trailing).

[...] When I compile this I get a warning :--
"string_utils.c:96: warning: function returns address of local
variable.

If isspace returns true, can the second conditional expression ever be
false?  If isspace returns false, the second expression won't be
evaluated.  Why is it there?

Explicitness, I guess.

seems like a bad reason

That's got to be the most repeated line in this group

"the correct prototype for main is..."
"this is off-topic for comp.lang.c"

Just because of setting all bits to zero?

I think he's commenting on setting all *bytes* to zero.
Remember calloc() initialises everything to zero.

You know what they say about
premature optimization...

this seems more like premature pessimization


--
Nick Keighley

The beginning of wisdom for a [software engineer] is to recognize the
difference between getting a program to work, and getting it right.
-- M A Jackson, 1975

 

[Ben Bacarisse]

This is becoming a FAQ.

Try:

#include <stdlib.h>
#include <string.h>
#include <ctype.h>

// Return value must be manually deallocated

char* xstrip(const char* str)
{
while (isspace((unsigned char) *str) && *str != '\0')
++str;

const char* end = str + strlen(str);
while ((isspace((unsigned char) *end) || *end == '\0') && end >
str)
--end;

char* buf = (char*) calloc(1, end - str + 2);

I'll try not to duplicate comments already made but to me the oddest
thing here is this rather complex loop condition and the +2. I know
why you do it, and it is safe, but it is also odd. In the case of
null string (or one with only spaces) you allocate two bytes rather
than one. This only matters if you worry about perplexing the
reader. It made me worry that there might be an unsafe off-by-one
problem somewhere else.

An rather unsung quality of code is code that makes the reader feel
safe. It reads well and always seems to be "just so". As soon and
peculiarity appears (even a safe one) that feeling is compromised.

To me the odd test, the + 2 and extra != '\0' further up are all safe
oddities that come into this bracket.

if (!buf)
return NULL;

strncpy(buf, str, end - str + 1);
return buf;
}

Or, if you want to modify the string in-place:

void xstrip(char* str)
{
char* start = str;

while (isspace((unsigned char) *start) && *start != '\0')
++start;

char* end = start + strlen(start);
while ((isspace((unsigned char) *end) || *end == '\0') && end >
start)
--end;

memmove((void*) str, (const void*) start, end - start + 1);
str[end - start + 1] = '\0';

But here the feeling of safety goes altogether. In the rare case that
str is a one-character null string, you write beyond the end of it.

 

[s0suk3]

This is becoming a FAQ.

<snip>









I'll try not to duplicate comments already made but to me the oddest
thing here is this rather complex loop condition and the +2.  I know
why you do it, and it is safe, but it is also odd.  In the case of
null string (or one with only spaces) you allocate two bytes rather
than one.  This only matters if you worry about perplexing the
reader.  It made me worry that there might be an unsafe off-by-one
problem somewhere else.

It's indeed an off-by-one error (even though nothing bad will happen
in this case). The confusion is that I assumed that 'str' and 'end'
would always be pointing to different characters (with 'end' pointing
to a character later in the string). If 'str' and 'end' end up
pointing to the same character (as will be the case for empty or all-
spaces strings), there will be an off-by-one error. This is fixed by
checking whether str == end, and allocating 1 extra byte in that case,
or otherwise allocating 2 extra bytes as the code currently does.

An rather unsung quality of code is code that makes the reader feel
safe.  It reads well and always seems to be "just so".  As soon and
peculiarity appears (even a safe one) that feeling is compromised.

To me the odd test, the + 2 and extra != '\0' further up are all safe
oddities that come into this bracket.

    if (!buf)
        return NULL;

    strncpy(buf, str, end - str + 1);
    return buf;
}

Or, if you want to modify the string in-place:

void xstrip(char* str)
{
    char* start = str;

    while (isspace((unsigned char) *start) && *start != '\0')
        ++start;

    char* end = start + strlen(start);
    while ((isspace((unsigned char) *end) || *end == '\0') && end >
start)
        --end;

    memmove((void*) str, (const void*) start, end - start + 1);
    str[end - start + 1] = '\0';

But here the feeling of safety goes altogether.  In the rare case that
str is a one-character null string, you write beyond the end of it.

And here the same off-by-one error does cause trouble

It should be:

if (start != end) {
memmove(str, start, end - start + 1);
str[end - start + 1] = '\0';
}
else
str[0] = '\0';

Sebastian

 

[srikar2097]

Thanks a lot guys; my problem is solved. Lots of valuable suggestions
here!!

Thanks to Keith Thompson for pointing me in the direction of FAQ's.

Srikar

This is becoming a FAQ.

I'll try not to duplicate comments already made but to me the oddest
thing here is this rather complex loop condition and the +2.  I know
why you do it, and it is safe, but it is also odd.  In the case of
null string (or one with only spaces) you allocate two bytes rather
than one.  This only matters if you worry about perplexing the
reader.  It made me worry that there might be an unsafe off-by-one
problem somewhere else.

It's indeed an off-by-one error (even though nothing bad will happen
in this case). The confusion is that I assumed that 'str' and 'end'
would always be pointing to different characters (with 'end' pointing
to a character later in the string). If 'str' and 'end' end up
pointing to the same character (as will be the case for empty or all-
spaces strings), there will be an off-by-one error. This is fixed by
checking whether str == end, and allocating 1 extra byte in that case,
or otherwise allocating 2 extra bytes as the code currently does.

An rather unsung quality of code is code that makes the reader feel
safe.  It reads well and always seems to be "just so".  As soon and
peculiarity appears (even a safe one) that feeling is compromised.

To me the odd test, the + 2 and extra != '\0' further up are all safe
oddities that come into this bracket.

    if (!buf)
        return NULL;
    strncpy(buf, str, end - str + 1);
    return buf;
}
Or, if you want to modify the string in-place:
void xstrip(char* str)
{
    char* start = str;
    while (isspace((unsigned char) *start) && *start != '\0')
        ++start;
    char* end = start + strlen(start);
    while ((isspace((unsigned char) *end) || *end == '\0') && end >
start)
        --end;
    memmove((void*) str, (const void*) start, end - start + 1);
    str[end - start + 1] = '\0';

But here the feeling of safety goes altogether.  In the rare case that
str is a one-character null string, you write beyond the end of it.

And here the same off-by-one error does cause trouble

It should be:

    if (start != end) {
        memmove(str, start, end - start + 1);
        str[end - start + 1] = '\0';
    }
    else
        str[0] = '\0';

Sebastian

 

[Default User]

Thanks a lot guys;

Please don't top-post. Your replies belong following or interspersed
with properly trimmed quotes. See the majority of other posts in the
newsgroup, or:
<http://www.caliburn.nl/topposting.html>