Compiler Warning

Discussion in 'C Programming' started by srikar2097, Dec 1, 2008.

  1. srikar2097

    srikar2097 Guest

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

    }
     
    srikar2097, Dec 1, 2008
    #1
    1. Advertising

  2. srikar2097

    Bartc Guest

    "srikar2097" <> wrote in message
    news:...
    >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.

    --
    Bartc
     
    Bartc, Dec 1, 2008
    #2
    1. Advertising

  3. srikar2097

    Guest

    On Dec 1, 3:55 pm, srikar2097 <> wrote:
    > 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.


    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.

    > }
     
    , Dec 1, 2008
    #3
  4. srikar2097

    Guest

    On Dec 1, 8:55 am, srikar2097 <> wrote:
    > 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
     
    , Dec 1, 2008
    #4
  5. srikar2097

    srikar2097 Guest

    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
    ()".




    On Dec 1, 7:22 pm, wrote:
    > On Dec 1, 3:55 pm, srikar2097 <> wrote:
    >
    >
    >
    >
    >
    > > 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.

    >
    > 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.
    >
    > > }
     
    srikar2097, Dec 1, 2008
    #5
  6. don't top-post. That is don't post your reply before the text you
    are replying to. I have re-arranged your post.

    On 1 Dec, 15:35, srikar2097 <> wrote:>
    > On Dec 1, 7:22 pm, wrote:
    > > On Dec 1, 3:55 pm, srikar2097 <> wrote:

    >
    > > > 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.


    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
    and <newline> \n.

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


    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)
     
    Nick Keighley, Dec 1, 2008
    #6
  7. srikar2097 <> writes:
    > 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,
    <http://www.c-faq.com/>, particularly questions 7.5a and 7.5b.

    > 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.

    --
    Keith Thompson (The_Other_Keith) <http://www.ghoti.net/~kst>
    Nokia
    "We must do something. This is something. Therefore, we must do this."
    -- Antony Jay and Jonathan Lynn, "Yes Minister"
     
    Keith Thompson, Dec 1, 2008
    #7
  8. On Mon, 1 Dec 2008 07:35:32 -0800 (PST), wrote:

    >On Dec 1, 8:55 am, srikar2097 <> wrote:
    >> 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')


    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


    --
    Remove del for email
     
    Barry Schwarz, Dec 2, 2008
    #8
  9. srikar2097

    Guest

    On Dec 1, 10:23 pm, Barry Schwarz <> wrote:
    > On Mon, 1 Dec 2008 07:35:32 -0800 (PST), wrote:
    > >On Dec 1, 8:55 am, srikar2097 <> wrote:
    > >> 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')

    >
    > 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.

    > >        ++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).


    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
     
    , Dec 2, 2008
    #9
  10. On 2 Dec, 06:38, wrote:
    > On Dec 1, 10:23 pm, Barry Schwarz <> wrote:
    > > On Mon, 1 Dec 2008 07:35:32 -0800 (PST), wrote:
    > > >On Dec 1, 8:55 am, srikar2097 <> wrote:


    > > >> 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.


    <snip>

    > > >Try:


    <snip>

    > > >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?

    >
    > Explicitness, I guess.


    seems like a bad reason


    > > >        ++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).

    >
    > 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"


    > > 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?


    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
     
    Nick Keighley, Dec 2, 2008
    #10
  11. writes:

    > On Dec 1, 8:55 am, srikar2097 <> wrote:
    >> I have written this small fn. to strip down whitespaces ("\n") a given
    >> string (only leading and trailing).


    This is becoming a FAQ.

    <snip>
    > 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.

    > }


    --
    Ben.
     
    Ben Bacarisse, Dec 2, 2008
    #11
  12. srikar2097

    Guest

    On Dec 2, 8:45 am, Ben Bacarisse <> wrote:
    > writes:
    > > On Dec 1, 8:55 am, srikar2097 <> wrote:
    > >> I have written this small fn. to strip down whitespaces ("\n") a given
    > >> string (only leading and trailing).

    >
    > This is becoming a FAQ.
    >
    > <snip>
    >
    >
    >
    > > 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.


    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
     
    , Dec 2, 2008
    #12
  13. srikar2097

    srikar2097 Guest

    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



    On Dec 2, 8:54 pm, wrote:
    > On Dec 2, 8:45 am, Ben Bacarisse <> wrote:
    >
    >
    >
    >
    >
    > > writes:
    > > > On Dec 1, 8:55 am, srikar2097 <> wrote:
    > > >> I have written this small fn. to strip down whitespaces ("\n") a given
    > > >> string (only leading and trailing).

    >
    > > This is becoming a FAQ.

    >
    > > <snip>

    >
    > > > 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.

    >
    > 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, Dec 4, 2008
    #13
  14. srikar2097

    Default User Guest

    Re: Compiler Warning - TPA

    srikar2097 wrote:

    > 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>
     
    Default User, Dec 4, 2008
    #14
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. sravan reddy

    warning when using design compiler

    sravan reddy, Aug 14, 2005, in forum: VHDL
    Replies:
    0
    Views:
    492
    sravan reddy
    Aug 14, 2005
  2. Pete Becker
    Replies:
    0
    Views:
    1,382
    Pete Becker
    Feb 10, 2005
  3. B. Williams

    warning C4267 and warning C4996

    B. Williams, Oct 26, 2006, in forum: C++
    Replies:
    17
    Views:
    2,640
  4. WARNING! Prosoftstore.com is a SCAM! WARNING!

    , Jul 8, 2007, in forum: ASP .Net Web Services
    Replies:
    0
    Views:
    326
  5. Julian Mehnle
    Replies:
    17
    Views:
    891
    Julian Mehnle
    May 18, 2006
Loading...

Share This Page