Using enum symbols as indices into menuentry options

PATCH In review Andreas Vogel 2 participants 4 messages Mar 3, 2012 1 message not in the archive

Share & tools

Andreas Vogel

author

2012-03-03 20:50 UTC #0 lore raw

PATCH 1/1 Using enum symbols as indices into menuentry options

Here is another tiny patch which uses a enum to reference the options array when dealing with command line options for a menuentry.

This method is already used by some other modules. I have some more patches which are adding more options for the menuentry command so finally readabilty will be improved using these symbols.

Any comments?

Andreas

------------------------------------------------------------
revno: 4099
committer: Andreas Vogel <[email protected]>
branch nick: 02-menuentry_options_enum
timestamp: Thu 2012-03-01 22:40:25 +0100
message:
* grub-core/commands/menuentry.c: use enum symbols for indexing the options array in order to improve readability
diff:
=== modified file 'grub-core/commands/menuentry.c'

commands/menuentry.c +16 -8

@@ -25,6 +25,14 @@
 #include <grub/i18n.h>
 #include <grub/normal.h>
 
+enum options
+  {
+    O_CLASS = 0,
+    O_USERS,
+    O_HOTKEY,
+    O_SOURCE,
+ };
+
 static const struct grub_arg_option options[] =
   {
     {"class", 1, GRUB_ARG_OPTION_REPEATABLE,

@@ -324,18 +332,18 @@
   if (! argc)
     return grub_error (GRUB_ERR_BAD_ARGUMENT, "missing arguments");
 
-  if (ctxt->state[3].set && ctxt->script)
+  if (ctxt->state[O_SOURCE].set && ctxt->script)
     return grub_error (GRUB_ERR_BAD_ARGUMENT, "multiple menuentry definitions");
 
-  if (! ctxt->state[3].set && ! ctxt->script)
+  if (! ctxt->state[O_SOURCE].set && ! ctxt->script)
     return grub_error (GRUB_ERR_BAD_ARGUMENT, "no menuentry definition");
 
   if (! ctxt->script)
     return grub_normal_add_menu_entry (argc, (const char **) args,
-				       (ctxt->state[0].set ? ctxt->state[0].args
-					: NULL), ctxt->state[1].arg,
-				       ctxt->state[2].arg, 0,
-				       ctxt->state[3].arg,
+				       (ctxt->state[O_CLASS].set ? ctxt->state[O_CLASS].args
+					: NULL), ctxt->state[O_USERS].arg,
+				       ctxt->state[O_HOTKEY].arg, 0,
+				       ctxt->state[O_SOURCE].arg,
 				       ctxt->extcmd->cmd->name[0] == 's');
 
   src = args[argc - 1];

@@ -350,8 +358,8 @@
     return grub_errno;
 
   r = grub_normal_add_menu_entry (argc - 1, (const char **) args,
-				  ctxt->state[0].args, ctxt->state[1].arg,
-				  ctxt->state[2].arg, prefix, src + 1,
+				  ctxt->state[O_CLASS].args, ctxt->state[O_USERS].arg,
+				  ctxt->state[O_HOTKEY].arg, prefix, src + 1,
 				  ctxt->extcmd->cmd->name[0] == 's');
 
   src[len - 1] = ch;

Vladimir 'φ-coder/phcoder' Serbinenko

2012-03-03 21:03 UTC #1 lore raw

commands/menuentry.c line 29

Quoted 2 lines · Andreas Vogel

commands/menuentry.c +2 -0
+enum options
+  {

Please don't define a type here, especially that a type "options" can easily conflict with something else.

Signature · 2 lines

Regards
Vladimir 'φ-coder/phcoder' Serbinenko

A message that is not in the archive (sent off-list or not yet received): [email protected]

Vladimir 'φ-coder/phcoder' Serbinenko

2012-03-03 21:24 UTC #3 lore raw

Keep list CC'ed.

Quoted 8 lines · Andreas Vogel

Quoted 6 lines · Andreas Vogel
commands/menuentry.c line 29
Quoted 2 lines
commands/menuentry.c +2 -0
+enum options
+  {
Please don't define a type here, especially that a type "options" can easily conflict with something else.
I reused that idea from commands/search_wrap.c and basically i liked that idea.

Just use an anonymous enum.

Signature · 2 lines

Regards
Vladimir 'φ-coder/phcoder' Serbinenko

Andreas Vogel

author

2012-03-03 21:57 UTC #4 lore raw

Quoted 4 lines

Keep list CC'ed.
Quoted 1 line · Andreas Vogel
Quoted 1 line · Andreas Vogel

Quoted 9 lines

Quoted 7 lines
Quoted 5 lines
Review
Quoted 2 lines
(quoted diff) +2 -0
+enum options
+  {
Please don't define a type here, especially that a type "options" can easily conflict with something else.
I reused that idea from commands/search_wrap.c and basically i liked that idea.
Just use an anonymous enum.

Please find attached the corrected patch using an anonymous enum.

=== modified file 'grub-core/commands/menuentry.c'

commands/menuentry.c +16 -8

@@ -25,6 +25,14 @@
 #include <grub/i18n.h>
 #include <grub/normal.h>
 
+enum
+  {
+    O_CLASS = 0,
+    O_USERS,
+    O_HOTKEY,
+    O_SOURCE,
+ };
+
 static const struct grub_arg_option options[] =
   {
     {"class", 1, GRUB_ARG_OPTION_REPEATABLE,

@@ -324,18 +332,18 @@
   if (! argc)
     return grub_error (GRUB_ERR_BAD_ARGUMENT, "missing arguments");
 
-  if (ctxt->state[3].set && ctxt->script)
+  if (ctxt->state[O_SOURCE].set && ctxt->script)
     return grub_error (GRUB_ERR_BAD_ARGUMENT, "multiple menuentry definitions");
 
-  if (! ctxt->state[3].set && ! ctxt->script)
+  if (! ctxt->state[O_SOURCE].set && ! ctxt->script)
     return grub_error (GRUB_ERR_BAD_ARGUMENT, "no menuentry definition");
 
   if (! ctxt->script)
     return grub_normal_add_menu_entry (argc, (const char **) args,
-				       (ctxt->state[0].set ? ctxt->state[0].args
-					: NULL), ctxt->state[1].arg,
-				       ctxt->state[2].arg, 0,
-				       ctxt->state[3].arg,
+				       (ctxt->state[O_CLASS].set ? ctxt->state[O_CLASS].args
+					: NULL), ctxt->state[O_USERS].arg,
+				       ctxt->state[O_HOTKEY].arg, 0,
+				       ctxt->state[O_SOURCE].arg,
 				       ctxt->extcmd->cmd->name[0] == 's');
 
   src = args[argc - 1];

@@ -350,8 +358,8 @@
     return grub_errno;
 
   r = grub_normal_add_menu_entry (argc - 1, (const char **) args,
-				  ctxt->state[0].args, ctxt->state[1].arg,
-				  ctxt->state[2].arg, prefix, src + 1,
+				  ctxt->state[O_CLASS].args, ctxt->state[O_USERS].arg,
+				  ctxt->state[O_HOTKEY].arg, prefix, src + 1,
 				  ctxt->extcmd->cmd->name[0] == 's');
 
   src[len - 1] = ch;