On Wed, Aug 27, 2025 at 10:07:35PM +0530, Sudhakar Kuppusamy wrote:
> Thank you Daniel for the review.
>
> > On 27 Aug 2025, at 9:11 PM, Daniel Kiper <[email protected]> wrote:
> > On Mon, Aug 25, 2025 at 04:38:33PM +0530, Sudhakar Kuppusamy wrote:

[...]

> >> +static grub_command_t cmd_verify, cmd_list_db, cmd_dbx_cert, cmd_db_cert;
> >> +
> >> +/* It registers the appended signatures GRUB commands. */
> >> +static void
> >> +register_appended_signatures_cmd (void)
> >> +{
> >> +  cmd_verify = grub_register_command ("append_verify", 
> >> grub_cmd_verify_signature, N_("SIGNED_FILE"),
> >> +                                      N_("Verify SIGNED_FILE against the 
> >> trusted X.509 certificates in the db list"));
> >> +  cmd_list_db = grub_register_command ("append_list_db", 
> >> grub_cmd_list_db, 0,
> >> +                                       N_("Show the list of trusted X.509 
> >> certificates from the db list"));
> >> +  cmd_db_cert = grub_register_command ("append_add_db_cert", 
> >> grub_cmd_db_cert, N_("X509_CERTIFICATE"),
> >> +                                       N_("Add trusted X509_CERTIFICATE 
> >> to the db list"));
> >> +  cmd_dbx_cert = grub_register_command ("append_rm_dbx_cert", 
> >> grub_cmd_dbx_cert, N_("X509_CERTIFICATE"),
> >> +                                        N_("Remove distrusted 
> >> X509_CERTIFICATE from the db list"));
> >> +}
> >> +
> >> +/* It unregisters the appended signatures GRUB commands. */
> >> +static void
> >> +unregister_appended_signatures_cmd (void)
> >> +{
> >> +  grub_unregister_command (cmd_verify);
> >> +  grub_unregister_command (cmd_list_db);
> >> +  grub_unregister_command (cmd_db_cert);
> >> +  grub_unregister_command (cmd_dbx_cert);
> >> +}
> >
> > I cannot seen any reason to have register_appended_signatures_cmd() and
> > unregister_appended_signatures_cmd() functions. Please put their code
> > directly into GRUB_MOD_INIT() and GRUB_MOD_FINI().
>
> Subsequent patches (patch 17) use these functions to register/unregister the 
> static and
> dynamic key support GRUB commands based on key management. That's why I kept 
> it as a separate functions.

I have checked the code. I think this is not needed. It should be enough
to change use_keystore flag. If it is not then probably something is wrong
in the code...

Daniel

_______________________________________________
Grub-devel mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/grub-devel

Reply via email to