3

I am trying to implement the rot13-algorithm in C. But since I am not very familiar with that language, I have some problems with my code right here.

Basically, I want to rotate every letter in args[] to 13 positions up. But this code seems to be pretty sluggish:

#include <stdio.h>

char[] rotate(char c[]) {
  char single;
  int i;
  int alen = sizeof(c)/sizeof(c[0]);
  char out[alen];

  for(i=0;i<=alen;i+=1) {
    if(c[i]>='a' && (c[i]+13)<='z'){
      out[i] = c[i]+13;
    }
  }

  return out;
}

int main(int argc, char *argv[]) {
  printf("The given args will be rotated\n");
  int i;
  char rotated[sizeof(argv)/sizeof(argv[0])];

  rotated = rotate(argv);

  /* printing rotated[] later on */
  return 0;

}

I know there a lot of holes here - could you show me how to fix this?

6
  • 3
    You might like to learn using a debugger (like gdb if you're on Linux) to step through your code, this help getting enlightend.
    – alk
    Nov 22, 2012 at 21:22
  • try to compile your code with gcc -Wall -Werror -pedantic -std=c99 (on linux) first :) Nov 22, 2012 at 21:29
  • 1
    I'd say it'll be sluggish, as in it'll never finish (or even start). char[] rotate(char c[]) { /* ... */ } won't compile because functions can't return arrays. Also you can't assign to an array as a whole (like in rotated = rotate(argv);). Maybe you can be more specific about you problem(s). Nov 22, 2012 at 21:36
  • sizeof does not give you the number of elements in an array. It gives you the size (in bytes) of an object's type. When you receive an array as a parameter, you're really just receiving a pointer to an array. The logical size of the array could be any size; there is no way to tell. After all, it's all just bytes in memory. Therefore, all that sizeof can tell you is the size of the pointer. Also, your code that does the actual rotation is not ROT13. Consider what happens when c[i] is 'n' or "greater." You may also want to review how argc and argv work. Nov 22, 2012 at 21:42
  • What do you mean with rotate? Nov 22, 2012 at 21:54

4 Answers 4

7

Thanks a lot guys, I solved the problem with this code

#include <stdio.h>

int rot13(int c){
  if('a' <= c && c <= 'z'){
    return rot13b(c,'a');
  } else if ('A' <= c && c <= 'Z') {
    return rot13b(c, 'A');
  } else {
    return c;
  }
}

int rot13b(int c, int basis){
  c = (((c-basis)+13)%26)+basis;
  return c;
}

int main() {
  printf("The given args will be rotated");
  int c;
  while((c = getchar()) != EOF){
    c = rot13(c);
    putchar(c);
  }
  return 0;
}
0
2

How @Michael said this char out[alen] is not accepted by the compiler because you can't declare an array size with a non constant value. Another problem of your code is the for loop for( i = 0; i < = alen; i+=1 ) the arrays start on 0 so if you do the for until the lenght's position you will be out of the array.

About the code:

  1. You must use a pointer to the start of the string as argument of the function, because You can't return arrays in C (But you can return pointers ).
  2. Your if( str[i] >= 'a' && (str[i]+13) <='z') is incorrect because you will convert some letters into symbols take a look.

________enter image description here --------------------------ASCII CODE!

    void rotate( char * str ) 
    {
        int i = 0;

        /* You do this until you find a '\0' */
        for( i = 0; str[ i ] != '\0' ; i++ ){

            /* Use the pointer notation if you passed a pointer. */
            /* If the letter is between a and m you can simply  sum it. */
            if( *( str + i ) >= 'a' && *( str + i ) < 'n')
                *( str + i ) += 13;       

            /* If the letter is between the n and z you have to do the opposite.*/
            else if( *( str + i ) >= 'n' && *( str + i ) <= 'z')
                *( str + i ) -= 13;
        }
    }
1

Size of arrays in C must be set at compile time, so you can't use non constant expression for array size.

Consider the below implementation:

// in place rotate
void rotate(char *str) 
// str must be a zero-terminated string
{
  int i =0;
  // loop until str itself is not NULL and str[i] is not zero 
  for(i=0;str && str[i]; ++i) // ++i is a pre-increment
  {
    if(str[i] >= 'a' && (str[i]+13) <='z')
    {
      str[i] = str[i]+13;       // modifying str in place
    }
  }
}

Then your main() can look like this:

int main(int argc, char *argv[]) 
{  
  printf("The given args will be rotated: %s\n", argv[1]);

  rotate(argv[1]);

  printf("Rotated: %s\n", argv[1]);
  return 0;
}

Update More advanced version of the transform that takes care of case when str[i] + 13 > 'z'

  for(i=0;str && str[i]; ++i) // ++i is a pre-increment
  {
      // ignore out of range chars
      if (str[i] < 'a' || str[i] > 'z') continue;
      // rotate 
      for (off = 13; off > ('z' - str[i]); ) 
      {
          off-= (1 + 'z' - str[i]);
          str[i] = 'a';
      }
      str[i]+=off;       
  }
3
  • as Antoine Mathys points and as pointed in my answer, the transform is not correct. However +1 for the i=0;str && str[i]; ++i Nov 22, 2012 at 22:12
  • Added solution for proper ROT13 that would also work for ROT(N) Nov 22, 2012 at 22:26
  • 2
    @MichaelSh Your claim that an array's size must be a compile-time constant is only true for C before C99, which introduced variable-length arrays. Although this recently got demoted to an optional feature in the current C11 standard.
    – reima
    Nov 22, 2012 at 22:32
1

This function can encode/decode to/from rot13 string. It's compatible with VIM's g? rot13 encoder.

void rot13 (char *s) {
    if (s == NULL)
        return;

    int i;
    for (i = 0; s[i]; i++) {
        if (s[i] >= 'a' && s[i] <= 'm') { s[i] += 13; continue; }
        if (s[i] >= 'A' && s[i] <= 'M') { s[i] += 13; continue; }
        if (s[i] >= 'n' && s[i] <= 'z') { s[i] -= 13; continue; }
        if (s[i] >= 'N' && s[i] <= 'Z') { s[i] -= 13; continue; }
    }
}

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.