| Message ID | 1276764159-28726-2-git-send-email-ext-andriy.shevchenko@nokia.com |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Jun 17, 2010 at 04:42, Shevchenko Andriy wrote: > --- a/flash_eraseall.c > +++ b/flash_eraseall.c > @@ -194,21 +194,21 @@ void process_options (int argc, char *argv[]) > { > int error = 0; > > + static const char *short_options = "jq"; > + static const struct option long_options[] = { > + {"help", no_argument, 0, 0}, > + {"version", no_argument, 0, 0}, > + {"jffs2", no_argument, 0, 'j'}, > + {"quiet", no_argument, 0, 'q'}, > + {"silent", no_argument, 0, 'q'}, > + > + {0, 0, 0, 0}, > + }; > + > exe_name = argv[0]; > > for (;;) { > int option_index = 0; > - static const char *short_options = "jq"; > - static const struct option long_options[] = { > - {"help", no_argument, 0, 0}, > - {"version", no_argument, 0, 0}, > - {"jffs2", no_argument, 0, 'j'}, > - {"quiet", no_argument, 0, 'q'}, > - {"silent", no_argument, 0, 'q'}, > - > - {0, 0, 0, 0}, > - }; > - > int c = getopt_long(argc, argv, short_options, > long_options, &option_index); > if (c == EOF) { NAK: no explanation why you're doing this, and current code has the variables scoped to where they actually get used. i wonder though why this code even bothers with "static". -mike
Mike Frysinger wrote: > NAK: no explanation why you're doing this, and current code has the > variables scoped to where they actually get used. > > i wonder though why this code even bothers with "static". The array is static to avoid compiling to code which fills in the array at runtime. I.e. it makes the code smaller, to the same size as if they were globals. And then, only because its static, the const can put them in the .rodata section, reducing unshared data size. Because they're static there's no benefit to moving them to another scope. -- Jamie
On Tue, Jun 22, 2010 at 20:23, Jamie Lokier wrote: > Mike Frysinger wrote: >> i wonder though why this code even bothers with "static". > > The array is static to avoid compiling to code which fills in the > array at runtime. I.e. it makes the code smaller, to the same size as > if they were globals. And then, only because its static, the const > can put them in the .rodata section, reducing unshared data size. if it were generated on the stack at runtime, the .text is shared too > Because they're static there's no benefit to moving them to another > scope. that's sort of what i expected, but i found it odd that it isnt: static const char * const short_options = "jq"; -mike
Mike Frysinger wrote: > On Tue, Jun 22, 2010 at 20:23, Jamie Lokier wrote: > > Mike Frysinger wrote: > >> i wonder though why this code even bothers with "static". > > > > The array is static to avoid compiling to code which fills in the > > array at runtime. I.e. it makes the code smaller, to the same size as > > if they were globals. And then, only because its static, the const > > can put them in the .rodata section, reducing unshared data size. > > if it were generated on the stack at runtime, the .text is shared too Shared, but larger than the static-const data it replaces. > > Because they're static there's no benefit to moving them to another > > scope. > > that's sort of what i expected, but i found it odd that it isnt: > static const char * const short_options = "jq"; You're right, I missed that. I'd have used this myself: static const char short_options[] = "jq"; -- Jamie
diff --git a/flash_eraseall.c b/flash_eraseall.c index a22fc49..f6319d1 100644 --- a/flash_eraseall.c +++ b/flash_eraseall.c @@ -194,21 +194,21 @@ void process_options (int argc, char *argv[]) { int error = 0; + static const char *short_options = "jq"; + static const struct option long_options[] = { + {"help", no_argument, 0, 0}, + {"version", no_argument, 0, 0}, + {"jffs2", no_argument, 0, 'j'}, + {"quiet", no_argument, 0, 'q'}, + {"silent", no_argument, 0, 'q'}, + + {0, 0, 0, 0}, + }; + exe_name = argv[0]; for (;;) { int option_index = 0; - static const char *short_options = "jq"; - static const struct option long_options[] = { - {"help", no_argument, 0, 0}, - {"version", no_argument, 0, 0}, - {"jffs2", no_argument, 0, 'j'}, - {"quiet", no_argument, 0, 'q'}, - {"silent", no_argument, 0, 'q'}, - - {0, 0, 0, 0}, - }; - int c = getopt_long(argc, argv, short_options, long_options, &option_index); if (c == EOF) {
Signed-off-by: Andy Shevchenko <ext-andriy.shevchenko@nokia.com> --- flash_eraseall.c | 22 +++++++++++----------- 1 files changed, 11 insertions(+), 11 deletions(-)