Error in Rot13 function

Why this dosen't work?

Code:

#include <stdio.h>

char* rot13(char*);

int main()
{
    char *string = "I'm a string";
    printf("%s", rot13(string));
    getchar();
    return 0;
}

char* rot13(char* string)
{
    unsigned int i, c, len = strlen(string);
    char ret[len];
    for (i=0; i < len; i++)
    {
        c = (int) string[i];
        if (c >= 65 && c <=77) c = c + 13;
        else if (c >= 78 && c <=90) c = c - 13;
        else if (c >= 97 && c <=109) c = c + 13; 
        else if (c >= 110 && c <=122) c = c - 13;
        ret[i] = (char) c;
    }
    return ret;
}

> Why this dosen't work?

Well

> unsigned int i, c, len = strlen(string);
Your string length is too short - you don't count the \0 at the end

> char ret[len];
I dunno what you used to compile this, but C doesn't have variable length arrays

> return ret;
And you can't return a pointer to a local variable, since it no longer exists when the function exits.

So how would I return?

this is what i do

Code:

#include <stdio.h>

char* rot13(char*);

int main()
{
    char string[] = "I'm a string";
    printf("%s", rot13(string));
    getchar();
    return 0;
}

char* rot13(char* string)
{
     int i, c, len=0;
    
    for(;string[len]!='\0';len++);
   static char *ret;
   ret=malloc(len);

    for (i=0; string[i]!='\0'; i++)
    {
        c = (int) string[i];
        if (c >= 65 && c <=77) c = c + 13;
        else if (c >= 78 && c <=90) c = c - 13;
        else if (c >= 97 && c <=109) c = c + 13; 
        else if (c >= 110 && c <=122) c = c - 13;
        ret[i] = (char) c;
    }
    ret[i]='\0';
    return ret;
}

/*output:
V'z n fgevat
*/

s.s.harish

what is this doing?

Code:

for(;string[len]!='\0';len++);

I beleive it's checking if the string isn't empty... Thanks ssharis

Why not something simpler, like this

Code:

#include <stdio.h>

char* rot13(char*);

int main()
{
    char string[] = "I'm a string";
    printf("%s\n", rot13(string));
    getchar();
    return 0;
}

char* rot13(char* string)
{
	int counter = 0;
	while (string[counter] != NULL)
	{
		string[counter] += 13;
		counter++;
	}
    return string;
}

Well all the answers are wrong.

s.s.harish managed to post both a buffer overflow and a memory leak at the same time - classic.

Madcow's answer mistakes NULL for '\0', and really doesn't implement rot13 at all (well it works on the first part of the alphabet, but not the second).

Code:

s.s.harish managed to post both a buffer overflow and a memory leak at the same time - classic.

i am veryy sorry for the wrong post salem. can u please tell me where i have gone wrong

s.s.harish

Well I would have a look at the values you are using here

Code:

for(;string[len]!='\0';len++);
   static char *ret;
   ret=malloc(len);

I don't see how you have done any better than the OP ie., you are not accounting for the '\0'

And did you remember to free() your malloc'd memory?

well, the intention of this statment is to find the length of the string

Code:

for(;string[len]!='\0';len++);

find the length of the string and allocate memory for the ret. what do u all thing about the above statment to find the length of the string rather than using strlen fucntion. which sometimes fails to return the proper length . please correct if i am wrong

and it was a mistake that i havn't free'd the ret

s.s.harish

Code:

#include <stdio.h>
#include <stdlib.h> /* for malloc */
#include <string.h> /* for strlen */
char* rot13(char*);

int main(void)
{
    char string[] = "I'm a string";
    char *answer = rot13(string);
    if ( answer != NULL ) {
      printf( "%s\n", answer );
      free( answer );
    }
    getchar();
    return 0;
}

char* rot13(char* string)
{
  size_t len = strlen( string );   /* better than for(;string[len]!='\0';len++); */
  char  *ret = malloc ( len + 1 ); /* allow for the \0 as well, and no need for static */

  if ( ret != NULL ) {  /* always check malloc returns */
    int i;
    for (i=0; string[i]!='\0'; i++)
    {
      char c = string[i];
      /* character constants are more readable, but it still assumes ASCII */
      /* In EBCDIC for example, 'A' + 13 isn't even a character */
      if (c >= 'A' && c <='M') c = c + 13;
      else if (c >= 'N' && c <='Z') c = c - 13;
      else if (c >= 'a' && c <='m') c = c + 13;
      else if (c >= 'n' && c <='z') c = c - 13;
      ret[i] = c;
    }
    ret[i]='\0';
  }
  return ret;
}

Perhaps with the static thing, you were thinking of this?

Code:

char *foo ( void ) {
  static char aReturnableArray[10];
  strcpy( aReturnableArray, "hello" );
  return aReturnableArray;
}

> which sometimes fails to return the proper length . please correct if i am wrong
I've never seen strlen() fail, unless it's because of some other mistake in the code.
Besides, how do you think strlen() is implemented - pretty much the way you've implemented it, so if strlen() is flawed, then so is your code.

You have found the length of the string well enough - but like strlen() you only counted the chars in the string, but not the '\0', so you might as well have done:

Code:

foo = strlen(string);
bar = malloc(foo + 1);  /* make room for '\0' */ 

thax very much for correcting me guys

Originally Posted by Salem

> char ret[len];
I dunno what you used to compile this, but C doesn't have variable length arrays

C99 allows it