2

For some reason this isn't working:

const char * str_reset_command = "\r\nReset";
const char * str_config_command = "\r\nConfig";

const char *commands[2] = {
    &str_reset_command,
    &str_config_command
};

Is this correct? (Is it code elsewhere that must be causing the problem?)

(I'm running it on a baseline 8bit microcontroller and have very limited debugging capabilities)

Clarification:

This is what I want to achieve:

const char * str_reset_command = "\r\nReset";
const char * str_config_command = "\r\nConfig";
const char * str_start_command = "\r\nStart";
const char * str_end_command = "\r\nEnd";

const char * const group0[1] = {
    str_reset_command
}
const char * const group1[2] = {
    str_reset_command,
    str_start_command
}
const char * const group2[3] = {
    str_reset_command,
    str_start_command,
    str_end_command
}
void doStuffToCharacter(unsigned char the_char){
    // ....
}
void doStuffToGroup(char ** group, unsigned char num_strings){
    for (unsigned char s=0; s < num_strings; s++){
        unsigned char idx = 0;
        while (group[s][idx]){
            doStuffToCharacter(group[s][idx]);
        }
    }
}
void main (void){
    doStuff(group0, 1);
    doStuff(group1, 2);
    doStuff(group2, 3);
}

As well as corrections, any neater suggestions of doing the above would be welcome.

Various combinations of the command strings need to be sent to a function for processing. All the strings will be in ROM, as will the groups of pointers to them.

Zword
  • 6,605
  • 3
  • 27
  • 52
Jodes
  • 14,118
  • 26
  • 97
  • 156
  • Question updated with clarifications – Jodes Feb 04 '14 at 19:46
  • Yesterday I have modified my answer, under the title "EDIT" (now I write the title "2nd TRY"). There, I explained a new code based on bit-masking. I think is a better approach, because it saves more memory, and there are not any use of arrays. Also, you have said that "various combinations of the command strings need to be sent". I think my approach is flexible for that purpose. There are several #define lines in this 2nd code, however this does not spend memory in the compiled program. – pablo1977 Feb 06 '14 at 05:12

7 Answers7

4

You created an array of pointers, but you are passing the addresses of pointers (meaning, a pointer to a pointer). what you should do is this-

    const char* commands[2]= {str_reset_command, str_config_command};
Yaniv Nikan
  • 108
  • 1
  • 7
3

Doesn't directly answer your question, but why not try this:

typedef enum {
    RESET_CMD,
    CONFIG_CND
} cmd_id;

const char *commands[2] = {
    "\r\nReset",
    "\r\nConfig"
};

And than use it like this:

commands[RESET_CMD]

EDIT

I'll rewrite your clarification to match this method:

typedef enum {
    RESET_CMD,
    CONFIG_CND,
    START_CMD,
    END_CMD
} cmd_id;

const char *commands[4] = {
    "\r\nReset",
    "\r\nConfig",
    "\r\nStart",
    "\r\nEnd"
};

const cmd_id group0[1] = {
    RESET_CMD
};

const cmd_id group1[2] = {
    RESET_CMD,
    START_CMD
};

const cmd_id group2[3] = {
    RESET_CMD,
    START_CMD,
    END_CMD
};

void doStuffToCharacter(unsigned char the_char){
    // ....
}

void doStuffToGroup(const cmd_id group[], unsigned char num_cmds){
    for (unsigned char s=0; s < num_cmds; s++) {
        unsigned char idx = 0;
        while (commands[group[s]][idx]) {
            doStuffToCharacter(commands[group[s]][idx++]);
        }
    }
}

void main (void){
    doStuff(group0, 1);
    doStuff(group1, 2);
    doStuff(group2, 3);
}
StoryTeller - Unslander Monica
  • 165,132
  • 21
  • 377
  • 458
  • After OP's clarification, this indeed seems like a very good way to do this. Defining groups would be simple too: `cmd_id group1[] = {RESET_CMD, CONFIG_CND};`. – user694733 Feb 05 '14 at 10:19
  • 1
    Fantastic, thank you! Now I can concentrate on re-growing lost hair! – Jodes Feb 05 '14 at 13:47
2

You have an array of const char pointers, and you try to store in it pointers to const char pointers. Remove & operators and it should work.

Wojtek Surowka
  • 20,535
  • 4
  • 44
  • 51
1

If you have variables declared outside of function your initializers must be constant. Const doesn't count in this case. You will make an array of pointers to pointers and then use the address of the char* variables.

const char * str_reset_command = "\r\nReset";
const char * str_config_command = "\r\nConfig";

const char **commands[2] = {
    &str_reset_command,
    &str_config_command
};

Don't forget to dereference the pointer when you use the array.

const char* string = *commands[1] ; //actually it is *(commands[1]) but the  [] operator has higher precedence anyway
this
  • 5,229
  • 1
  • 22
  • 51
1

Try this. It's not quite what you want, but it does the job.

#define RESET_COMMAND             "Reset"
#define CONFIG_COMMAND            "Config"  

const char *commands[2] = {  
    RESET_COMMAND,  
    CONFIG_COMMAND,  
};  

int  
main() {  
   ...  
}
No One in Particular
  • 2,846
  • 4
  • 27
  • 32
  • Thanks, but I would prefer something which the compiler will definitely optimise so that the strings will not be duplicated in ROM when they're included in multiple groups – Jodes Feb 04 '14 at 19:56
  • @Jodes Have you actually confirmed that compiler will duplicate strings if you use same literals on multiple occasions on same compilation unit? It is very easy for compilers to detect this and all compilers I have worked with so far optimize it. – user694733 Feb 05 '14 at 10:06
1

You are using global variables. Are you really sure you need this approach?

By following your code as is, you defined str_reset_command as a const char *. This means that is a pointer to char, which also has the qualifier const. You have not to confuse "const" with const. :)
The compiler consider "const" to every expression build from expression involving literals.
On the other hand, the const qualifier means that you are defining a "variable" whose value cannot be modified, except in the very moment of its definition.

The expression "\r\nReset" is a "const" because is a string literal.
However const char * str_reset_command is not a "const", but a variable not able to be modified.

The difference is that the literals are "constants" for the compiler, because it can calculate their value in compiling time. However a const object is held as a variable, because its value may be determined in execution time. For example, consider the const parameters of several functions in <string.h>.

Your variable group0 is defined as a pointer to an array of char.
Since it is defined as a global variable, its initializer has to be a "const" (in compile time sense) value. But you have provided str_reset_command, which is not a literal (or an expression involving only literals). Thus, its value cannot be determined in compiling time and it cannot be used as initialize there.

Maybe you could try to write an initializer function:

 const char * str_reset_command = "\r\nReset";
 const char * str_config_command = "\r\nConfig";
 const char * str_start_command = "\r\nStart";
 const char * str_end_command = "\r\nEnd";

 char * group0[1];
 char * group1[2];
 char * group2[3];

 void initGroups(void) {
      group2[0] = group1[0] = group0[0] = str_reset_command;
      group2[1] = group1[1] = str_start_command;
      group2[2] = str_end_command;
 }

 int main(void) {
    initGroups();

    // Do stuff...
 }

I dropped the const qualifier since now we need to modify the pointers.

Anyway, the use of global variables has some side effects.
Try to modify this aspect, if possible.


EDIT

2nd TRY

I was thinking in your program, and I has changed completely your approach.
First of all, I don't understand why you are using all that "group[]" arrays.
Since you want to optimize memory resources, this is not optimal.
On the other hand, your "constant strings" seems to be only a few.
By assuming that they are not more than 8, the information of the strings involved in a given group can be hold in 1 byte.
I mean, you can define a "group" or a "set" by means of the bits of a byte, thus putting the bit 1 (on) if a given member belongs to the set, and 0 else.

Besides, since you accept the use of arrays of pointers to constant chars, I think that all your strings can be held in an array of constant char at the beggining of the code. This can be held in ROM, by means of a const declaration.

You have used names as str_reset, str_start, and so on.
It seems that you need this information to be clear for yourself.
This information can be preserved in the code by means of compiler-constants and/or enumerations, which have not any cost in the compiled program.

I have designed a code that use bit-masking.
This let you use up to 8 strings.
Since a bit-mask is a power of 2, this could not be used as an array index.
However, one can define in a very consequent way a list of enumaration constants with names going "in parallel" with the bit-mask constants.
IN particular, you will be capable or change the order of the enumeration, but your code will keep working.

To complete the picture, I have used a feature of C99/C11 standard (that you seems to use, because the way you are declaring the for statements), that allows us to initialize indidivual members of an array by writting the desired index.
Since the indexes now will have names given by an enumeration, you can trust in this technique, without pay attention to the actual indexes that are been used.

I have used <stdio.h> and printf() just to test the program. You can erase these lines.

#include <stdio.h>

#define bit0 0x01u  /* Binary 0000 0001 */
#define bit1 0x02u  /* Binary 0000 0010 */
#define bit2 0x04u  /* Binary 0000 0100 */
#define bit3 0x08u  /* Binary 0000 1000 */
#define bit4 0x10u  /* Binary 0001 0000 */
#define bit5 0x20u  /* Binary 0010 0000 */
#define bit6 0x40u  /* Binary 0100 0000 */
#define bit7 0x80u  /* Binary 1000 0000 */

enum {reset_command = 0, config_command, start_command, end_command};

#define bit_reset  (1u << reset_command)    /*  Equal to bit0 */
#define bit_config (1u << config_command)   /*  Equal to bit1 */
#define bit_start  (1u << start_command)    /*  Equal to bit2 */
#define bit_end    (1u << end_command)      /*  Equal to bit3 */

const char * const str[] = {
         [reset_command]  = "\r\nReset",
         [config_command] = "\r\nConfig",
         [start_command]  = "\r\nStart",
         [end_command]    = "\r\nEnd"
    };

const unsigned char bitgroup0 = bit_reset;
const unsigned char bitgroup1 = bit_reset | bit_start;
const unsigned char bitgroup2 = bit_reset | bit_start | bit_end;

void doStuffToCharacter(unsigned char the_char){
    printf("%c", the_char);
}

void doStuff(const char * const * str, unsigned char bitgroup){
    printf("\n\nGroup: %hu\n", bitgroup);

    for (unsigned char b=bitgroup, j=0; b; b >>= 1u, j++){
        if (b & 1u)  {
           for(unsigned char idx = 0; str[j][idx]; idx++) {
                 doStuffToCharacter(str[j][idx]);
           }
        }
    }
}
int main (void){
    doStuff(str, bitgroup0);
    doStuff(str, bitgroup1);
    doStuff(str, bitgroup2);
}

As you can see, each "group" now needs only 1 byte. (Your approach used at least 1, 2 and 3 bytes).
The number of iterations in the for() statement does not exceed the greatest bit "on" in the groupbit parameter.

The requirement of having const char object held in const pointers is also fullfilled.

The size of the array is automatically determined by the compiler to hold the maximum index.

The for() loop iterates over a "byte" initialized to the "group" you passed as a parameter.
The last bit is tested against 1.
If this bit is 1, then some operation is done.
Else, this is skipped in it goes to the next iteration.
The last bit is dropped with the assignment b >>= 1u.

With each iteration, we need to walk away the index j of the array str.
Thus, the j-th bit is 1 if and only if the j-string is processed.
Next, every is repeated, until b has not more bits 1.

If you use "groups" whose bits 1 are all contiguous, then the program works in the same way that you expected in your example.

Now you are able to choose the operations you want, just switching the appropiated bits.

pablo1977
  • 4,281
  • 1
  • 15
  • 41
  • Thank you for your detailed answer! What collateral effects would global variables have? When using global variables, I believe the compiler may make desirable optimisations by not passing variables unnecessarily between functions, saving some precious RAM and execution cycles on the microcontroller – Jodes Feb 05 '14 at 09:45
  • 1
    "Collateral effects" means "side effects" (I had in trouble with translation to English). It is referred to a common logic error in programming. In a modular language as C, it is good practice that each function keeps hidden its own variables and communicate information to the rest of program through parameters or returning values. In your particular case, if you really need to use global variables, you must be very carefull in order to not convert your program in a mess. – pablo1977 Feb 05 '14 at 10:07
  • @Jodes: I have added a code that avoids your approach of multiple arrays, keeps the strings in ROM memory, and saves memory. Take a look and tell me if you like it. – pablo1977 Feb 05 '14 at 11:46
1

Your array declaration is inconsistent with the prototype of the function that uses it:

const char *commands[2] = {  // <- declared as array of 2 pointers to char
doStuffToGroup(char ** group // <- used as pointer to pointer to char

It is a common C/C++ pitfall (though vectors used as a remplacement for vanilla arrays make that far less common in C++).

See this answer for a more detailed explanation.

Community
  • 1
  • 1
kuroi neko
  • 8,479
  • 1
  • 19
  • 43