Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clean up command handling in hdp.c/h #433

Merged
merged 2 commits into from
Sep 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 55 additions & 33 deletions mfhdf/dumper/hdp.c
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,34 @@
* help@hdfgroup.org. *
* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */

#define HDP_MASTER
#define VSET_INTERFACE
#include "hdp.h"
#include "local_nc.h" /* to use some definitions */

/********************/
/* Global Variables */
/********************/

/* indicates Vsets have been initialized for the current file */
intn vinit_done = FALSE;

/********************************/
/* Local variables and typedefs */
/********************************/

/* hdp commands (stored as (value, name) pairs to keep them in sync) */
typedef enum { HELP, LIST, DUMPSDS, DUMPRIG, DUMPVG, DUMPVD, DUMPGR, BAD_COMMAND } command_value_t;

typedef struct command_t {
const command_value_t value;
const char *name;
} command_t;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tying the enum value and string together into a struct ensures they will never be out of sync


static const command_t commands[] = {{HELP, "help"}, {LIST, "list"},
{DUMPSDS, "dumpsds"}, {DUMPRIG, "dumprig"},
{DUMPVG, "dumpvg"}, {DUMPVD, "dumpvd"},
{DUMPGR, "dumpgr"}, {BAD_COMMAND, "BADNESS - not a valid command"}};

/* Print the usage message about this utility */
static void
usage(intn argc, char *argv[])
Expand Down Expand Up @@ -87,58 +110,56 @@ init_dump_opts(dump_info_t *dump_opts)
int
main(int argc, char *argv[])
{
command_t cmd; /* command to perform */
intn curr_arg; /* current cmd line argument */
dump_opt_t glob_opts; /* global options for all commands */
intn j; /* local counting variables */
command_value_t cmd = BAD_COMMAND; /* command to perform */
intn curr_arg; /* current cmd line argument */
dump_opt_t glob_opts; /* global options for all commands */
intn j; /* local counting variables */

memset(&glob_opts, 0, sizeof(dump_opt_t));

if (argc < 2) {
usage(argc, argv);
exit(1);
} /* end if */
exit(EXIT_FAILURE);
}

curr_arg = 1;
/* printf("Argument 0: %s\n",argv[0]);
printf("Argument 1: %s\n",argv[1]);
*/
while (curr_arg < argc && (argv[curr_arg][0] == '-')) {
/* while(curr_arg<argc && (argv[curr_arg][0]=='-' || argv[curr_arg][0]=='/')) { */
switch (argv[curr_arg][1]) {
case 'H':
/* case 'h': */ /* Print help for a given command */
/* case 'h': */ /* Print help for a given command */
if (curr_arg < argc - 1) {
glob_opts.help = TRUE; /* for displaying options. */
break;
}
default:
usage(argc, argv); /* Display the general usage. */
exit(1);
} /* end switch */
exit(EXIT_FAILURE);
}
curr_arg++;
} /* end while */
}

for (j = 0, cmd = HELP; j < (sizeof(commands) / sizeof(const char *)); j++, cmd++) {
if (HDstrcmp(argv[curr_arg], commands[j]) == 0)
for (j = 0; j < (sizeof(commands) / sizeof(command_t)); j++) {
if (HDstrcmp(argv[curr_arg], commands[j].name) == 0) {
cmd = commands[j].value;
break;
} /* end for */
}
}
if (cmd == BAD_COMMAND) {
printf("Invalid command: %s\n", argv[curr_arg]);
exit(EXIT_FAILURE);
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before we printed the value of cmd (in code below), which would just be the sentinel NONE value, which is unhelpful.


/* printf("cmd=%d\n",(int)cmd);
printf("command=%s\n",argv[curr_arg]);
*/
curr_arg++;

/* must be a legit command */
switch (cmd) {
case LIST:
if (FAIL == do_list(curr_arg, argc, argv, glob_opts.help))
exit(1);
exit(EXIT_FAILURE);
break;

case DUMPSDS:
if (FAIL == do_dumpsds(curr_arg, argc, argv, glob_opts.help))
exit(1);
exit(EXIT_FAILURE);
break;

case DUMPRIG:
Expand All @@ -148,36 +169,37 @@ main(int argc, char *argv[])
fprintf( stderr, ">>> Please make a note that dumprig is no longer available.\n");
fprintf( stderr, " The command dumpgr is and should be used in its place.\n" );
if (FAIL == do_dumpgr(curr_arg, argc, argv, glob_opts.help)) */
exit(1);
exit(EXIT_FAILURE);
break;

case DUMPVG:
if (FAIL == do_dumpvg(curr_arg, argc, argv, glob_opts.help))
exit(1);
exit(EXIT_FAILURE);
break;

case DUMPVD:
if (FAIL == do_dumpvd(curr_arg, argc, argv, glob_opts.help))
exit(1);
exit(EXIT_FAILURE);
break;

case DUMPGR:
if (FAIL == do_dumpgr(curr_arg, argc, argv, glob_opts.help))
exit(1);
exit(EXIT_FAILURE);
break;

case HELP:
case NONE:
usage(argc, argv);
break;

/* Should not get here - invalid commands are handled earlier */
case BAD_COMMAND:
default:
printf("Invalid command!, cmd=%d\n", (int)cmd);
exit(1);
printf("Invalid command!\n");
exit(EXIT_FAILURE);
break;
} /* end switch */
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Printing the enum value was unhelpful. Printing the string is better, which we do above.

}

return (0);
return EXIT_SUCCESS;
}

/* -----------------------------------------------------------------
Expand Down
25 changes: 4 additions & 21 deletions mfhdf/dumper/hdp.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,15 @@
#include "hfile.h"

/* Global Variables */
#ifndef HDP_MASTER
extern
#endif /* !HDP_MASTER */
intn vinit_done
#ifdef HDP_MASTER
= FALSE /* indicates Vsets have been init'ed for the current file */
#endif /* HDP_MASTER */
;
extern intn vinit_done;

/* Global Definitions */
#define MAXCHOICES 50

#ifndef MAXNAMELEN
#define MAXNAMELEN 100
#endif /* !MAXNAMELEN */
#endif

#define MAXCLASSLEN 100
#define MAXPERLINE 65 /* max # of chars per line in the output */
#define MAXRANK 100
Expand Down Expand Up @@ -283,18 +278,6 @@ extern
} \
}

/* Add enum and string for new commands to both of the variables below. */
/* Preserve the correct/corresponding ordering */
typedef enum { HELP, LIST, DUMPSDS, DUMPRIG, DUMPVG, DUMPVD, DUMPGR, NONE } command_t;
#ifndef HDP_MASTER
extern
#endif /* !HDP_MASTER */
const char *commands[]
#ifdef HDP_MASTER
= {"help", "list", "dumpsds", "dumprig", "dumpvg", "dumpvd", "dumpgr"}
#endif /* HDP_MASTER */
;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved into hdp.c since it's not used anywhere else


/* Global options structure */
typedef struct {
intn help; /* Print help on this command */
Expand Down