C - you should use a variable array of function pointers
This question is about how to solve my problem at the level of how I design my program. For a school project, I am creating a wrapper that has several built in functionality. One of these purposes of the function (cmd_type) is to check if the given argument is listed in this function list. Here's a partial implementation:
int cmd_type(int argc, char *argv[]) {
if (argc == 2) {
for (int i = 0; i < BUILTIN_FUNC_COUNT; i++) {
if (strcmp(cmds_name[i], argv[1]) == 0) {
printf("%s is a shell builtin\n", argv[1]);
return 0; // found it
}
}
// still need to search path, call stat(path/cmd)
errmsg("not implemented! type", 1);
} else {
err_msg("type", 1);
}
}
Defining if statements for every function my shell supports sounds like a bad choice because the list can expand over time and I still need to keep a list of function names. Therefore, initially I planned to define an array of function names and an array of their pointers, for example:
char cmds_name[BUILTIN_FUNC_COUNT-1][16];
char (*cmds_ptr)(int,*char[])[BUILTIN_FUNC_COUNT-1];
// make list of built-in funcs
strcpy(cmds_name[0], "exit");
strcpy(cmds_name[1], "cd");
// make list of func pointers
cmds_ptr[0] = &cmd_exit;
cmds_ptr[1] = &cmd_cd;
They can be accessed like this:
// try builtin cmds
for (int i = 0; i < BUILTIN_FUNC_COUNT; i++) {
if (strcmp(cmds_name[i], argv[0]) == 0) {
last_cmd_err = (*cmds_ptr[i])(argc, argv);
continue; // we found it, so next loop
}
}
Then they will happily take (int argc, char * argv []) as arguments. But cmd_path () needs list access in addition to these arguments, so I would have to define it as a global or define a global pointer to it ... While researching this, I found this answer saying that this kind of approach was really bad style: Stackoverflow
So my questions are: is this a good way to solve this problem, or should I just do if / else-statements / is a better way? Would you recommend a global pointer to an array of function names?
source to share
I'm going to suggest structure
from cmd_name
and function pointer
as follows:
typedef struct{
char cmds_name[16];
char (*cmds_ptr)(int,*char[]);
} cmd_type;
Now define a table of static
this type for all of yours cmds
:
static const cmd_type cmd_table[] = {
{"exit", &cmd_exit},
{"cd", &cmd_cd},
.......
.......
};
Finally, do the following:
for (int i = 0; i < BUILTIN_FUNC_COUNT; i++) {
if (strcmp(cmd_table[i].cmds_name, argv[0]) == 0) {
last_cmd_err = (*cmd_table[i].cmds_ptr)(argc, argv);
continue; // we found it, so next loop
}
}
The decision to choose between if-else and the global table is a matter of personal taste and coding style. I would prefer the above solution simply because it improves code readability and reduces clutter. There may be other constraints in your environment that might influence your decision - for example, if no table entries are huge and there is a limit on the global memory space - the if-else route would be the best choice.
NTN!
source to share
I wouldn't go with operators if-else
. There is nothing wrong in the decision (2)
proposed in fooobar.com/questions/2419940 / ... .
You can have a table with a row and a function to serve the record:
typedef struct cmd_desc
{
char cmd[80];
int builtin_cmd(int argc, char **argv, void *extra);
} CMD_DESC;
static CMD_DESC descTable[] =
{
{ "exit", cmd_exit },
{ "cd", cmd_cd },
{ "$ON_OPEN_CMD", OnOpenCmd },
{ "$OPEN_EXTRA_CMD", OpenExtraCmd },
{ "$AC", ActionCmd },
{ "$AD", ActionDataCmd },
{ "$EC", ExtraCmd },
{ "$TC", TextCmd },
{ "", NULL }
};
int cmd_exit (int argc, char **argv, void *extra)
{
//...
}
Access / Execution:
for (int tokenIndex=0; strcmp(descTable[tokenIndex].cmd,""); tokenIndex++) //search table
{
if ( strcmp( (descTable[tokenIndex]).cmd, argv[0] ) == 0 )
{
int ret = (*(descTable[tokenIndex]).builtin_cmd( argc, argv, extra);
}
}
I have used the above approach in my applications and it worked well for me.
The table can be easily expanded and the readability of the table is better than the if else chaining.
source to share