2

Hey guys i have two problems in my c code. I have created a structure named rotor:

typedef struct rotor {
    char characters[26];
    int rotationPos;
} rotor;

I want to create an instance of that struct like this:

        rotor r;
[error] r.characters[26]= {'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K',
        'L', 'M', 'N', 'O', 'P', 'Q', 'R', 'S', 'T', 'U', 'V', 'W', 'X', 'Y',
        'Z'};
    r.rotationPos = 5;

and it gives me the following error: "-Syntax erro; -Expected expression before '{'"

i also have this function

void rotate(rotor r) {
    char aux[26];
    int i;
    for (i = 0; i < 26; i++) {
        if (i == 0) {
[error]         aux[26] = r->characters[i];
        } else
[error]         aux[i] = r->characters[i + 1];
    }
[error] r->characters=aux;
}

that gives: "invalid type argument of '->' (have 'rotor')" Can you guys tell me what am i doing wrong please? Thanks!

Miguel Morujão
  • 1,131
  • 1
  • 14
  • 39
  • declare void rotate(rotor *r) to use pointer or void rotate(rotor &r) to hide the pointer (then use r.characters). – nopsoft Apr 25 '14 at 20:04
  • To initialize let's try: rotor r = {characters:{'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K', 'L', 'M', 'N', 'O', 'P', 'Q', 'R', 'S', 'T', 'U', 'V', 'W', 'X', 'Y', 'Z'}, rotationPos:5}; – nopsoft Apr 25 '14 at 20:10

4 Answers4

4

You can't initialize your array like that once you've already created a rotor object, because you can't assign to arrays after you define them. You have to do something like this:

rotor r = {{'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K',
        'L', 'M', 'N', 'O', 'P', 'Q', 'R', 'S', 'T', 'U', 'V', 'W', 'X', 'Y',
        'Z'}, 5};

For the same reason, this:

r->characters=aux

at the end of your function is never going to work, because you can't assign one array to another. You'll have to copy all the members, either manually, or using something like memcpy().

For the second question, you declare r as rotor, not as a pointer to rotor, so you should use the . operator, not the -> operator, which is for pointers to structs. Alternatively, change your function to accept a pointer, and pass the address of your struct to it (which is usually better than passing copies or large structs around).

Crowman
  • 25,242
  • 5
  • 48
  • 56
  • if i use the "." instead of "->" i get the error: request for member ‘characters’ in something not a structure or union – Miguel Morujão Apr 25 '14 at 20:19
  • You'll need to post updated code, preferably something short and complete that will compile, since `characters` clearly is a member of `struct rotor` in the example you give. – Crowman Apr 25 '14 at 20:22
1

Since rotor is a struct, the character array is created along with it. It's not a pointer to a character array, so you can't point it to something else. You can use the syntax Paul mentioned, or you can overwrite the current array.

    char chars[26] = { 'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K', 'L', 'M',
                       'N', 'O', 'P', 'Q', 'R', 'S', 'T', 'U', 'V', 'W', 'X', 'Y', 'Z' };
    rotor r;
    memcpy(r.characters, chars, sizeof(chars));

Also, when you call the rotate method, rotor will be copied on to the stack. You'll perform the actual rotation, but the struct you performed it on will be lost when you return. You should pass a pointer instead.

void rotate(rotor *r);

Call it using the & operator.

rotate(&r);

In that method you'll also run into the issue of not being able to replace the array. I'll leave that as an exercise to you, but please comment back if you need any further help. (Since rotor is a pointer inside this method, you'll want to use the -> accessor syntax.)

Welcome to Stack Overflow!

wjl
  • 7,143
  • 1
  • 30
  • 49
1

1) You try to use an initialization list to a char, you meant to write

 r.characters= {'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K',
    'L', 'M', 'N', 'O', 'P', 'Q', 'R', 'S', 'T', 'U', 'V', 'W', 'X', 'Y',
    'Z'};

but this is not valid C.

edit: as pointed by WhozCraig, prefer memcpy (I forgot the null terminator with strcpy)

char chars[26] = { 'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K', 'L', 'M',
                   'N', 'O', 'P', 'Q', 'R', 'S', 'T', 'U', 'V', 'W', 'X', 'Y', 'Z' };
rotor r;
memcpy(r.characters, chars, sizeof(chars));

or

rotor r = {{'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K',
        'L', 'M', 'N', 'O', 'P', 'Q', 'R', 'S', 'T', 'U', 'V', 'W', 'X', 'Y',
        'Z'}, 5};

2) Use are using the -> operator on a rotor object, which is not a pointer. Replace it with "."

void rotate(rotor r) {
    char aux[26];
    int i;
    for (i = 0; i < 26; i++) {
        if (i == 0) {
            aux[26] = r.characters[i];
        }
        else
        {
            aux[i] = r.characters[i + 1];
        }
    }
    memcpy(r.characters, aux, sizeof(aux));
}

Your rotate method is suspicious. What is the intended usage ? Edit: OK

quantdev
  • 23,517
  • 5
  • 55
  • 88
  • I would advise against `strcpy()`, as you'll be writing a 27th char (the terminator) into an array only 26 chars wide, and thus invoking UB. Similarly, `aux[26] = ...` is out of bounds, also invoking UB. – WhozCraig Apr 25 '14 at 20:19
  • well i want to make every letter go a positions further, if 'A' is in characters[0] it goes to the last position (characters[26]) if 'B' was in the second position it goes now to the the first one, etc... – Miguel Morujão Apr 25 '14 at 20:26
  • @MMrj How you want to do what you want to do is relevant. My only point is there are four methods in this code, and two of them invoke undefined behavior. Why it is the "accepted" answer is beyond me. – WhozCraig Apr 25 '14 at 20:31
  • WhozCraig: absolutely, this is why I asked "Your rotate method is suspicius. What is the intended usage ?" – quantdev Apr 25 '14 at 20:37
  • I don't think it's the rotate method that's suspicious, as much as it is that the first option in your answer is a syntax error, the second invokes undefined behavior, and the third is copied from another answer here. – Crowman Apr 25 '14 at 20:38
  • if (i == 0) { aux[25] = r->characters[i]; aux[0] = r->characters[i + 1]; } my if was all messed up, i fixed it – Miguel Morujão Apr 25 '14 at 20:43
  • Ok MMj, got it (edited). Paul's answer should have been accepted. – quantdev Apr 25 '14 at 20:43
0

As you already set a size in your structure, you can't do an assignation as you do here.

Plus, you should add a '\0' at the end of your char array if you want to read it properly and avoid reading other char of your memory

You can either declare a array of char with its size:

  char array[4] = {'1', '2', '3', '\0'};

Of if you already declared it:

   char array[4];
   strcpy(array, "123\0");

You can read more about memory allocation and the C string functions.

This post should also help you: Assigning strings to arrays of characters

Community
  • 1
  • 1
tcollart
  • 1,032
  • 2
  • 17
  • 29
  • 1
    Adding a terminating `'\0'` is only needed/useful if you intend to treat the `char` array as a string, rather than a regular array. If you don't, it doesn't need to be, and probably shouldn't be, added. – Crowman Apr 25 '14 at 20:20