Velocity Reviews > String Reverse Question

# String Reverse Question

Rakesh
Guest
Posts: n/a

 04-16-2004
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.

Guest
Posts: n/a

 04-16-2004

"Rakesh" <(E-Mail Removed)> a écrit dans le message de
news:(E-Mail Removed) om...
> 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.

Rakesh
Guest
Posts: n/a

 04-17-2004
"Régis Troadec" <(E-Mail Removed)> wrote in message news:<c5ppkr\$uer\$(E-Mail Removed)>...
> 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.

Christopher Benson-Manica
Guest
Posts: n/a

 04-17-2004
Rakesh <(E-Mail Removed)> 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
Guest
Posts: n/a

 04-17-2004
Rakesh <(E-Mail Removed)> 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.

Martin Ambuhl
Guest
Posts: n/a

 04-17-2004
Christopher Benson-Manica wrote:
> Rakesh <(E-Mail Removed)> 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++.

Al Bowers
Guest
Posts: n/a

 04-17-2004

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) {

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: http://www.velocityreviews.com/forums/(E-Mail Removed) (remove the x to send email)
http://www.geocities.com/abowers822/

Robert Bachmann
Guest
Posts: n/a

 04-17-2004
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
--
(E-Mail Removed) |) |)
http://rb.rbdev.net |\.|).

Joe Wright
Guest
Posts: n/a

 04-17-2004
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 (E-Mail Removed)
"Everything should be made as simple as possible, but not simpler."
--- Albert Einstein ---

Ben Pfaff
Guest
Posts: n/a

 04-17-2004
Joe Wright <(E-Mail Removed)> 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.