Prathamesh Chavan <pc44800@xxxxxxxxx> writes: > * The function get_submodule_displaypath() was modified for the case > when get_super_prefix() returns a non-null value. The condition to check > if the super-prefix ends with a '/' is removed. To accomodate this change > appropriate value of super_prefix is passed instead in the recursive calls > of init_submodule() and status_submodule(). OK. > * To accomodate the possiblity of a direct call to the function > init_submodule(), a callback function init_submodule_cb() is introduced > which takes cache_entry and init_cb structures as input params, and > calls init_submodule() with parameters which are more appropriate > for a direct call of this function. OK. > * Similar changes were even done for status_submodule(). But as it was > observed that the number of params increased a lot due to flags > like quiet, recursive, cached, etc, and keeping in mind the future > subcommand's ported functions as well, a single unsigned int called > cb_flags was introduced to store all of these flags, instead of having > parameter for each one. Use of a flag word instead of many bit parameter is a very good idea. I do not think it is brilliant to call a field in a structure that is used as an interface to the callback interface cb_flags, though, especially when it is already clear that it is about the callback from the name the structure. Just name them "flags". I may or may not leave more detailed comments on individual patches. Thanks.
