Re: [PATCH 01/10] scripts/basic/bin2c: Complete error handling in main()
2016-10-28 17:31 GMT+09:00 SF Markus Elfring: > From: Markus Elfring > Date: Thu, 27 Oct 2016 16:15:04 +0200 > > Return values were not checked from five calls of the function "printf". > > This issue was detected also by using the Coccinelle software. > > > * Add a bit of exception handling there. > > * Optimise this function implementation a bit. > > - Replace two output calls with the functions "fputs" and "puts". > > - Use the preincrement operator for the variable "total". > > Signed-off-by: Markus Elfring > --- > scripts/basic/bin2c.c | 26 -- > 1 file changed, 16 insertions(+), 10 deletions(-) > > diff --git a/scripts/basic/bin2c.c b/scripts/basic/bin2c.c > index c3d7eef..c6c8860 100644 > --- a/scripts/basic/bin2c.c > +++ b/scripts/basic/bin2c.c > @@ -8,29 +8,35 @@ > */ > > #include > +#include > > int main(int argc, char *argv[]) > { > int ch, total = 0; > > if (argc > 1) > - printf("const char %s[] %s=\n", > - argv[1], argc > 2 ? argv[2] : ""); > + if (printf("const char %s[] %s=\n", > + argv[1], argc > 2 ? argv[2] : "") < 16) > + return errno; > > do { > - printf("\t\""); > + if (fputs("\t\"", stdout) < 0) > + return errno; > + > while ((ch = getchar()) != EOF) { > - total++; > - printf("\\x%02x", ch); > - if (total % 16 == 0) > + if (printf("\\x%02x", ch) < 4) > + return errno; > + if (++total % 16 == 0) > break; > } > - printf("\"\n"); > + > + if (puts("\"") < 0) > + return errno; Is replacing printf("\"\n") with puts("\"") optimization? Frankly, the result of this patch seems extremely unreadable code. -- Best Regards Masahiro Yamada
Re: [PATCH 01/10] scripts/basic/bin2c: Complete error handling in main()
2016-10-28 17:31 GMT+09:00 SF Markus Elfring : > From: Markus Elfring > Date: Thu, 27 Oct 2016 16:15:04 +0200 > > Return values were not checked from five calls of the function "printf". > > This issue was detected also by using the Coccinelle software. > > > * Add a bit of exception handling there. > > * Optimise this function implementation a bit. > > - Replace two output calls with the functions "fputs" and "puts". > > - Use the preincrement operator for the variable "total". > > Signed-off-by: Markus Elfring > --- > scripts/basic/bin2c.c | 26 -- > 1 file changed, 16 insertions(+), 10 deletions(-) > > diff --git a/scripts/basic/bin2c.c b/scripts/basic/bin2c.c > index c3d7eef..c6c8860 100644 > --- a/scripts/basic/bin2c.c > +++ b/scripts/basic/bin2c.c > @@ -8,29 +8,35 @@ > */ > > #include > +#include > > int main(int argc, char *argv[]) > { > int ch, total = 0; > > if (argc > 1) > - printf("const char %s[] %s=\n", > - argv[1], argc > 2 ? argv[2] : ""); > + if (printf("const char %s[] %s=\n", > + argv[1], argc > 2 ? argv[2] : "") < 16) > + return errno; > > do { > - printf("\t\""); > + if (fputs("\t\"", stdout) < 0) > + return errno; > + > while ((ch = getchar()) != EOF) { > - total++; > - printf("\\x%02x", ch); > - if (total % 16 == 0) > + if (printf("\\x%02x", ch) < 4) > + return errno; > + if (++total % 16 == 0) > break; > } > - printf("\"\n"); > + > + if (puts("\"") < 0) > + return errno; Is replacing printf("\"\n") with puts("\"") optimization? Frankly, the result of this patch seems extremely unreadable code. -- Best Regards Masahiro Yamada
Re: [PATCH 01/10] scripts/basic/bin2c: Complete error handling in main()
On Fri, Oct 28, 2016 at 1:31 AM, SF Markus Elfringwrote: > From: Markus Elfring > Date: Thu, 27 Oct 2016 16:15:04 +0200 > > Return values were not checked from five calls of the function "printf". > > This issue was detected also by using the Coccinelle software. > > > * Add a bit of exception handling there. > > * Optimise this function implementation a bit. The most interesting thing about this patch was trying to figure out how to actually get bin2c to run at all. Making a defconfig kernel didn't run it. Making a kernel with the latest Ubuntu 16.10 config file didn't run it. Setting CONFIG_IKCONFIG runs it (once), for the folks who want to use scripts/extract-ikconfig. After that, if you dig about in the makefiles, it looks like you have to turn on the Tomoyo LSM -- which doesn't seem to be a common occurrence -- or else set CONFIG_KEXEC_FILE to generate the 'purgatory' thing it uses. Again, not the most frequent of events, as far as I can tell. Given how uncommon running bin2c seems to be, "optimizing" it may not be a useful project. -- Jim
Re: [PATCH 01/10] scripts/basic/bin2c: Complete error handling in main()
On Fri, Oct 28, 2016 at 1:31 AM, SF Markus Elfring wrote: > From: Markus Elfring > Date: Thu, 27 Oct 2016 16:15:04 +0200 > > Return values were not checked from five calls of the function "printf". > > This issue was detected also by using the Coccinelle software. > > > * Add a bit of exception handling there. > > * Optimise this function implementation a bit. The most interesting thing about this patch was trying to figure out how to actually get bin2c to run at all. Making a defconfig kernel didn't run it. Making a kernel with the latest Ubuntu 16.10 config file didn't run it. Setting CONFIG_IKCONFIG runs it (once), for the folks who want to use scripts/extract-ikconfig. After that, if you dig about in the makefiles, it looks like you have to turn on the Tomoyo LSM -- which doesn't seem to be a common occurrence -- or else set CONFIG_KEXEC_FILE to generate the 'purgatory' thing it uses. Again, not the most frequent of events, as far as I can tell. Given how uncommon running bin2c seems to be, "optimizing" it may not be a useful project. -- Jim