String Reverse Question

Discussion in 'C Programming' started by Rakesh, Apr 16, 2004.

  1. Rakesh

    Rakesh Guest

    Hi,
    I have this function to reverse the given string.
    I am just curious if that is correct and there could be better way of
    doing it / probable bugs in the same.

    The function prototype is similar to the one in any standard C
    library.


    <---- Code starts -->

    char * my_strrev(char * mine) {
    static char * p = NULL; // To make sure that the pointer is not
    lost on return.
    char * q;
    int len;
    int i;

    len = strlen(mine);
    p = new char[len + 1];
    // 1 for '\0' character

    q = mine;
    i = len - 1;
    while (*q) {
    *(p + i) = *q++;
    i--;
    }
    *(p + len ) = '\0';
    return p;
    }

    <-- Code Ends -->

    - Rakesh.
     
    Rakesh, Apr 16, 2004
    #1
    1. Advertising

  2. "Rakesh" <> a écrit dans le message de
    news:...
    > Hi,


    Hi,

    > I have this function to reverse the given string.
    > I am just curious if that is correct and there could be better way of
    > doing it / probable bugs in the same.
    >
    > The function prototype is similar to the one in any standard C
    > library.
    >
    >
    > <---- Code starts -->
    >
    > char * my_strrev(char * mine) {
    > static char * p = NULL; // To make sure that the pointer is not
    > lost on return.
    > char * q;
    > int len;
    > int i;
    >
    > len = strlen(mine);


    mine could be NULL and cause a segfault. Be also careful that mine is null
    terminated.

    You can put this above the strlen call :
    if (mine==NULL) return NULL;

    > p = new char[len + 1];


    new ?? it looks like C++, not C. Use malloc, calloc, or realloc instead.

    p = malloc(len+1);

    if (p)
    {

    > // 1 for '\0' character
    >
    > q = mine;
    > i = len - 1;
    > while (*q) {
    > *(p + i) = *q++;
    > i--;
    > }
    > *(p + len ) = '\0';


    }

    > return p;
    > }
    >
    > <-- Code Ends -->


    Regis
    >
    > - Rakesh.
     
    Régis Troadec, Apr 17, 2004
    #2
    1. Advertising

  3. Rakesh

    Rakesh Guest

    "Régis Troadec" <> wrote in message news:<c5ppkr$uer$>...
    > mine could be NULL and cause a segfault. Be also careful that mine is null
    > terminated.


    Oh Yeah - Thanks a ton. This is very important. But just a design q.
    here - am i supposed to handle this one / should i leave it to the
    user of the library to make sure that NULL is not passed to this.
    Which one seems the best.


    >
    > You can put this above the strlen call :
    > if (mine==NULL) return NULL;
    >
    > > p = new char[len + 1];

    >
    > new ?? it looks like C++, not C. Use malloc, calloc, or realloc instead.


    Oops ! My mistake . Of late, I had been doing a lot in C++ and
    hence the problem.

    Thanks for your comments, Régis
     
    Rakesh, Apr 17, 2004
    #3
  4. Rakesh <> spoke thus:

    > char * my_strrev(char * mine) {

    const char *mine

    > static char * p = NULL; // To make sure that the pointer is not
    > lost on return.


    1) Don't post C++ style comments to comp.lang.c (more later)
    2) Misplaced concern - the pointer value is returned and thus is
    not lost.

    > char * q;


    Harmless but unnecessary - you can simply use mine rather than
    declaring another variable.

    > int len;
    > int i;


    > len = strlen(mine);
    > p = new char[len + 1];


    This (new) is C++. Don't post it here. ITYM

    p=malloc( len*sizeof(*p)+1 );

    > // 1 for '\0' character


    You did well to remember it.

    > q = mine;
    > i = len - 1;
    > while (*q) {
    > *(p + i) = *q++;
    > i--;
    > }
    > *(p + len ) = '\0';
    > return p;
    > }


    --
    Christopher Benson-Manica | I *should* know what I'm talking about - if I
    ataru(at)cyberspace.org | don't, I need to know. Flames welcome.
     
    Christopher Benson-Manica, Apr 17, 2004
    #4
  5. Rakesh <> spoke thus:

    > Oh Yeah - Thanks a ton. This is very important. But just a design q.
    > here - am i supposed to handle this one / should i leave it to the
    > user of the library to make sure that NULL is not passed to this.
    > Which one seems the best.


    Most of the standard string.h functions dislike NULL pointers, so
    having your code do likewise (and documenting that fact) seems
    reasonable. (I missed this issue - oops.)

    --
    Christopher Benson-Manica | I *should* know what I'm talking about - if I
    ataru(at)cyberspace.org | don't, I need to know. Flames welcome.
     
    Christopher Benson-Manica, Apr 17, 2004
    #5
  6. Christopher Benson-Manica wrote:
    > Rakesh <> spoke thus:


    >> static char * p = NULL; // To make sure that the pointer is not
    >>lost on return.


    > 1) Don't post C++ style comments to comp.lang.c (more later)


    The real concern is not that "//..." are C++ style comments. As of C99,
    they are C comments as well. The problem is shown by your quotation;
    the comment is split by either his posting software or your reading
    software. This makes the code uncompilable without going through and
    pasting comments back together. This problem suggests that not only
    posters to comp.lang.c but also posters to comp.lang.c++ ought not use
    that style comment. After all, "/* ... */" comments are still legal in C++.
     
    Martin Ambuhl, Apr 17, 2004
    #6
  7. Rakesh

    Al Bowers Guest

    Rakesh wrote:
    > Hi,
    > I have this function to reverse the given string.
    > I am just curious if that is correct and there could be better way of
    > doing it / probable bugs in the same.
    >
    > The function prototype is similar to the one in any standard C
    > library.


    In addition to the excellent help you good from others, I would
    like to add the following.

    >
    >
    > <---- Code starts -->
    >
    > char * my_strrev(char * mine) {


    Since the string argument will not be altered, I would make it:
    char *my_strrev(const char *mine)

    > static char * p = NULL; // To make sure that the pointer is not
    > lost on return.
    > char * q;
    > int len;


    The strlen function returns type size_t. Although your int type for
    len may not fail, it would be best to make the this:
    size_t len;

    > int i;
    >
    > len = strlen(mine);
    > p = new char[len + 1];
    > // 1 for '\0' character
    >
    > q = mine;
    > i = len - 1;
    >

    Opps! You need to consider how you want to handle the
    situation, should a user of the function call it with an
    empty string,ie., my_strrev("");. Here len will have
    the value of 0, and in the expression i = len - 1;, i
    will have the value of -1.

    while (*q) {
    > *(p + i) = *q++;


    Here is the "Opps!". With i have value of -1, the code
    *(p + i) becomes *(p - 1)

    > i--;
    > }
    > *(p + len ) = '\0';
    > return p;
    > }
    >


    I would suggest that you put in the function a check testing
    len for 0 and take appropriate action.

    Example:

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

    char *my_strrev(const char *str)
    {
    char *rev = NULL,*s1;
    size_t len = strlen(str);

    if(len != 0 && (rev = malloc(len+1)) != NULL)
    for(*(rev+len) = '\0',s1=(char *)str;len;s1++,len--)
    *(rev+(len-1)) = *s1;
    return rev;
    }

    int main(void)
    {
    char *s,str[] = "This is a long string";

    if((s = my_strrev("This is a long string")) != NULL)
    printf("The string \"%s\"\nreversed is \"%s\"\n",str,s);
    else printf("The string \"%s\" was not reversed\n",str);
    free(s);
    return 0;
    }

    --
    Al Bowers
    Tampa, Fl USA
    mailto: (remove the x to send email)
    http://www.geocities.com/abowers822/
     
    Al Bowers, Apr 17, 2004
    #7
  8. Al Bowers wrote:
    [snip]
    > char *my_strrev(const char *str)
    > {
    > char *rev = NULL,*s1;
    > size_t len = strlen(str);
    >
    > if(len != 0 && (rev = malloc(len+1)) != NULL)


    Why (len != 0)?
    If I invoke my_strrev("") I would expect it
    to return a pointer to '\0' and not a NULL-pointer.

    > for(*(rev+len) = '\0',s1=(char *)str;len;s1++,len--)
    > *(rev+(len-1)) = *s1;
    > return rev;
    > }


    Best regards,
    Robert Bachmann
    --
    |) |)
    http://rb.rbdev.net |\.|).
     
    Robert Bachmann, Apr 17, 2004
    #8
  9. Rakesh

    Joe Wright Guest

    Rakesh wrote:

    > Hi,
    > I have this function to reverse the given string.
    > I am just curious if that is correct and there could be better way of
    > doing it / probable bugs in the same.
    >
    > The function prototype is similar to the one in any standard C
    > library.
    >
    >
    > <---- Code starts -->
    >
    > char * my_strrev(char * mine) {
    > static char * p = NULL; // To make sure that the pointer is not
    > lost on return.
    > char * q;
    > int len;
    > int i;
    >
    > len = strlen(mine);
    > p = new char[len + 1];
    > // 1 for '\0' character
    >
    > q = mine;
    > i = len - 1;
    > while (*q) {
    > *(p + i) = *q++;
    > i--;
    > }
    > *(p + len ) = '\0';
    > return p;
    > }
    >
    > <-- Code Ends -->
    >
    > - Rakesh.


    You're kidding, right?

    char *revstr(char *s) {
    char t, *b = s, *e = s + strlen(s) -1;
    if (s) while (b < e) t = *b, *b++ = *e, *e-- = t;
    return s;
    }

    Maybe it's more difficult in C++. :-(
    --
    Joe Wright mailto:
    "Everything should be made as simple as possible, but not simpler."
    --- Albert Einstein ---
     
    Joe Wright, Apr 17, 2004
    #9
  10. Rakesh

    Ben Pfaff Guest

    Joe Wright <> writes:

    > char *revstr(char *s) {
    > char t, *b = s, *e = s + strlen(s) -1;
    > if (s) while (b < e) t = *b, *b++ = *e, *e-- = t;
    > return s;
    > }
    >
    > Maybe it's more difficult in C++. :-(


    In C++ it's easier:
    std::reverse(s, strchr(s, '\0'));
    --
    Go not to Usenet for counsel, for they will say both no and yes.
     
    Ben Pfaff, Apr 17, 2004
    #10
  11. <posted & mailed>

    The code you posted won't compile with a C compiler. Here is a valid C
    version:

    #include <stdlib.h>
    #include <string.h>
    char * strrev (const char *s)
    {
    char * p;
    int i, j;
    j = strlen(s)
    if (p = calloc(1, j+1))
    for (i=0; i<j; ++i)
    p = s[j-i];
    return p;
    }

    Rakesh wrote:

    > Hi,
    > I have this function to reverse the given string.
    > I am just curious if that is correct and there could be better way of
    > doing it / probable bugs in the same.
    >
    > The function prototype is similar to the one in any standard C
    > library.
    >
    >
    > <---- Code starts -->
    >
    > char * my_strrev(char * mine) {
    > static char * p = NULL; // To make sure that the pointer is not
    > lost on return.
    > char * q;
    > int len;
    > int i;
    >
    > len = strlen(mine);
    > p = new char[len + 1];
    > // 1 for '\0' character
    >
    > q = mine;
    > i = len - 1;
    > while (*q) {
    > *(p + i) = *q++;
    > i--;
    > }
    > *(p + len ) = '\0';
    > return p;
    > }
    >
    > <-- Code Ends -->
    >
    > - Rakesh.


    --
    remove .spam from address to reply by e-mail.
     
    James McIninch, Apr 17, 2004
    #11
  12. Joe Wright a écrit :
    > char *revstr(char *s) {
    > char t, *b = s, *e = s + strlen(s) -1;
    > if (s) while (b < e) t = *b, *b++ = *e, *e-- = t;
    > return s;
    > }


    Just some questions about this code:

    1) What happens if s is a null pointer? Is strlen(s) supposed to work?
    2) If e value is (s - 1), isn't the comparison (b < e) an undefined
    behaviour, as e is no more pointing to an object or one element past
    that object?

    --
    Richard
     
    Richard Delorme, Apr 17, 2004
    #12
  13. On Sat, 17 Apr 2004 18:27:47 GMT, James McIninch
    <> wrote:

    ><posted & mailed>
    >
    >The code you posted won't compile with a C compiler. Here is a valid C
    >version:


    Only for strange definitions of valid. It neither compiles nor
    performs after the syntax error is fixed.

    >
    >#include <stdlib.h>
    >#include <string.h>
    >char * strrev (const char *s)


    Names starting with str followed by a lower case letter are reserved.

    >{


    Let's assume that s points to an array containing "ab".

    > char * p;
    > int i, j;
    > j = strlen(s)


    j would be set to 2 if there were a semicolon at the end of the
    statement.

    > if (p = calloc(1, j+1))
    > for (i=0; i<j; ++i)


    For the first iteration through the loop, i is 0.

    > p = s[j-i];


    For the first iteration, this is p[0] = s[2]. We already know that
    s[2] is '\0' so p will forever point to a string of length 0.

    Additionally, since i never equals j, s[0] is never copied to p.

    > return p;
    >}
    >




    <<Remove the del for email>>
     
    Barry Schwarz, Apr 17, 2004
    #13
  14. On Sat, 17 Apr 2004 20:50:26 +0200, Richard Delorme <>
    wrote:

    >Joe Wright a écrit :
    >> char *revstr(char *s) {
    >> char t, *b = s, *e = s + strlen(s) -1;
    >> if (s) while (b < e) t = *b, *b++ = *e, *e-- = t;
    >> return s;
    >> }

    >
    >Just some questions about this code:
    >
    >1) What happens if s is a null pointer? Is strlen(s) supposed to work?


    strlen does not support receiving a NULL pointer. Behavior would be
    undefined at this point.

    >2) If e value is (s - 1), isn't the comparison (b < e) an undefined
    >behaviour, as e is no more pointing to an object or one element past
    >that object?


    This can only happen is s points to a string whose first char is '\0'
    but if that happens then you are correct about the undefined behavior.



    <<Remove the del for email>>
     
    Barry Schwarz, Apr 17, 2004
    #14
  15. "Rakesh" <> a écrit dans le message de
    news:...
    > "Régis Troadec" <> wrote in message

    news:<c5ppkr$uer$>...
    > > mine could be NULL and cause a segfault. Be also careful that mine is

    null
    > > terminated.

    >
    > Oh Yeah - Thanks a ton. This is very important. But just a design q.
    > here - am i supposed to handle this one / should i leave it to the
    > user of the library to make sure that NULL is not passed to this.
    > Which one seems the best.


    It depends on your own requirements : would you like the user of your
    functions take care about null pointers and memory handling or not ?
    As Christopher said, most of standard functions of <string.h> need to pay
    careful attention with null pointers. I think it's fine to code in the same
    way as well I think it's fine to prevent any memory access violation. If it
    were for business, I would implement my functions according to the given
    requirements. For my personal stuff, I always handle null pointers.

    Finally, I missed to propose you my implementation of string reversing, here
    it is :

    <code>

    char * my_strrev(const char * src)
    {
    size_t len, i, j;
    char * dst;

    if (src==NULL) return NULL;

    len = strlen(src);

    if ((dst = malloc(len+1)) != NULL)
    {

    if (len % 2) dst[len/2] = src[len/2];
    for(i=0, j=len-1; i<len/2; ++i, --j)
    {
    dst = src[j]; dst[j] = src;
    }
    dst[len] = '\0';
    }

    return dst;
    }

    </code>

    Regis

    >
    >
    > >
    > > You can put this above the strlen call :
    > > if (mine==NULL) return NULL;
    > >
    > > > p = new char[len + 1];

    > >
    > > new ?? it looks like C++, not C. Use malloc, calloc, or realloc instead.

    >
    > Oops ! My mistake . Of late, I had been doing a lot in C++ and
    > hence the problem.
    >
    > Thanks for your comments, Régis
     
    Régis Troadec, Apr 17, 2004
    #15
  16. Joe Wright <> writes:
    [...]
    > char *revstr(char *s) {
    > char t, *b = s, *e = s + strlen(s) -1;
    > if (s) while (b < e) t = *b, *b++ = *e, *e-- = t;
    > return s;
    > }


    You're checking whether s is null *after* passing it to strlen().

    --
    Keith Thompson (The_Other_Keith) <http://www.ghoti.net/~kst>
    San Diego Supercomputer Center <*> <http://users.sdsc.edu/~kst>
    Schroedinger does Shakespeare: "To be *and* not to be"
     
    Keith Thompson, Apr 18, 2004
    #16
  17. Christopher Benson-Manica <> writes:
    [...]
    > This (new) is C++. Don't post it here. ITYM
    >
    > p=malloc( len*sizeof(*p)+1 );


    (where p was declared as a char*).

    If we're going to assume that p is a char*, the sizeof is superfluous;
    it's simpler and clearer to write:

    p = malloc(len + 1);

    That's a reasonable assumption given that we're allocating extra space
    for a '\0' character.

    On the other hand, if we want to allow for the possibility that the
    declaration of p could be changed so it points to something bigger
    than one byte (perhaps a wchar_t), we'd want to use:

    p = malloc(len * (sizeof *p + 1));

    to allocate one additional object rather than one additional byte.

    --
    Keith Thompson (The_Other_Keith) <http://www.ghoti.net/~kst>
    San Diego Supercomputer Center <*> <http://users.sdsc.edu/~kst>
    Schroedinger does Shakespeare: "To be *and* not to be"
     
    Keith Thompson, Apr 18, 2004
    #17
  18. "Keith Thompson" <> a écrit dans le message de
    news:...

    Hi,

    [snipped]

    > On the other hand, if we want to allow for the possibility that the
    > declaration of p could be changed so it points to something bigger
    > than one byte (perhaps a wchar_t), we'd want to use:
    >
    > p = malloc(len * (sizeof *p + 1));
    >
    > to allocate one additional object rather than one additional byte.


    I guess you meant p = malloc( (len+1)*sizeof*p ); to allocate one
    additional object.

    Regards,
    Regis

    > --
    > Keith Thompson (The_Other_Keith)

    <http://www.ghoti.net/~kst>
    > San Diego Supercomputer Center <*>

    <http://users.sdsc.edu/~kst>
    > Schroedinger does Shakespeare: "To be *and* not to be"
     
    Régis Troadec, Apr 18, 2004
    #18
  19. "Régis Troadec" <> writes:
    [...]
    > It depends on your own requirements : would you like the user of your
    > functions take care about null pointers and memory handling or not ?
    > As Christopher said, most of standard functions of <string.h> need to pay
    > careful attention with null pointers.


    Actually, most of the standard functions in <string.h> can ignore the
    issue of null pointers altogether. Passing a null pointer to strlen()
    invokes undefined behavior.

    If you're going to have your own string functions check for null
    pointers, you need to decide how to handle the error. Since C doesn't
    have exceptions, there's no obvious way to indicate the error.

    --
    Keith Thompson (The_Other_Keith) <http://www.ghoti.net/~kst>
    San Diego Supercomputer Center <*> <http://users.sdsc.edu/~kst>
    Schroedinger does Shakespeare: "To be *and* not to be"
     
    Keith Thompson, Apr 18, 2004
    #19
  20. Rakesh

    Al Bowers Guest

    Robert Bachmann wrote:
    > Al Bowers wrote:
    > [snip]
    >
    >> char *my_strrev(const char *str)
    >> {
    >> char *rev = NULL,*s1;
    >> size_t len = strlen(str);
    >>
    >> if(len != 0 && (rev = malloc(len+1)) != NULL)

    >
    >
    > Why (len != 0)?
    > If I invoke my_strrev("") I would expect it
    > to return a pointer to '\0' and not a NULL-pointer.


    If there is an empty string argument on the above defined function,
    then there would not be a dynamic allocation and NULL is returned.

    If the OP purpose was to do as you expected, then the OP
    code will fail. The example was a demo of correcting this flaw,
    without changing the OP's use of a dynamic allocation.

    I too would do what you expect. But doing this in a function, which
    would dynamically allocate memory for no useful purpose (there
    are no characters to reverse), is a waste of resources.
    That is not to consider the resources of deallocation should
    you feel the need.

    To do what you expect, I would design the function to have
    two char point arguments, one pointing to the source string, and
    the other pointing to storage that is declared/allocated by the
    calling function. Therefore, there would be no need for the
    my_strrev function to make allocations. The return value will
    simply point to the dest storage where a empty string will reside.

    >
    >> for(*(rev+len) = '\0',s1=(char *)str;len;s1++,len--)
    >> *(rev+(len-1)) = *s1;
    >> return rev;
    >> }

    >


    I'm surprised that you did not catch the boner in
    function main. :).

    --
    Al Bowers
    Tampa, Fl USA
    mailto: (remove the x to send email)
    http://www.geocities.com/abowers822/
     
    Al Bowers, Apr 18, 2004
    #20
    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. Curt_C [MVP]
    Replies:
    0
    Views:
    1,986
    Curt_C [MVP]
    Jan 22, 2004
  2. Roedy Green
    Replies:
    9
    Views:
    1,650
    Jeff Schwab
    Aug 11, 2005
  3. dogbite
    Replies:
    4
    Views:
    729
    osmium
    Oct 10, 2003
  4. ssecorp
    Replies:
    47
    Views:
    1,086
    Default User
    Aug 8, 2008
  5. Mitch Tishmack

    utf8 string with reverse question

    Mitch Tishmack, Aug 15, 2005, in forum: Ruby
    Replies:
    5
    Views:
    263
    Levin Alexander
    Aug 15, 2005
Loading...

Share This Page