On Sunday 12 April 2009 00:11:45 phcoder wrote: > Updated. Same changelog > > >> + { > >> + update_val (grub_strcmp (args[*argn], args[*argn + 2]) == 0); > >> + (*argn) += 3; > > > > I myself feel that these parentheses are redundant, but I don't know how > > others think. For C programmers, it is well known that * has a very high > > priority. > > These parenthesis are necessary if doing sth like > (*argn)++ > since ++ and += are logically and visually similar I prefer to put the > parenthesis
OK. > > > Getting tired, so I skip the same criticism from here. > > Actually it would have been enough to say "same applies further on in > patch" > > >> + if (*argn + 1 < argc && !grub_strcmp (args[*argn], "-s")) > >> + { > >> + grub_file_t file; > >> + file = grub_file_open (args[*argn + 1]); > >> + update_val (file && grub_file_size (file)); > > > > This is not very safe, because grub_file_size returns grub_off_t which is > > a 64-bit unsigned int. By converting it into 32-bit signed int > > implicitly, the result can be zero, even when the size is not zero. So it > > is better to say explicitly, != 0. BTW, I think you can simplify test_parse. For example, you write "if (*argn + 2 < argc ...)" many times, but it should be possible to test this condition only once per loop. Regards, Okuji _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel