Re: [PATCH 1/5] mkeficapsule: constify function parameters

On 6/16/23 13:34, Stefan Herbrechtsmeier wrote:

From: Malte Schmidt malte.schmidt@weidmueller.com

Thanks for considering which parameters may be constants.

nits:

The Urban Dictionary defines 'constify' as:

"To constantly do something, like constantly watching anime all day."

%s/constify function parameters/make function parameters const/

Use const keyword for function parameters where appropriate.

Signed-off-by: Malte Schmidt malte.schmidt@weidmueller.com Signed-off-by: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com

---

tools/mkeficapsule.c | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/tools/mkeficapsule.c b/tools/mkeficapsule.c index 52be1f122e..b8db00b16b 100644 --- a/tools/mkeficapsule.c +++ b/tools/mkeficapsule.c @@ -88,8 +88,8 @@ static void print_usage(void)

• are filled in by create_auth_data().

*/ struct auth_context {

• char *key_file;
• char *cert_file;

• const char *key_file;
• const char *cert_file; uint8_t *image_data; size_t image_size; struct efi_firmware_image_authentication auth;

@@ -112,7 +112,7 @@ static int dump_sig;

• • 0 - on success
• • -1 - on failure

*/ -static int read_bin_file(char *bin, uint8_t **data, off_t *bin_size) +static int read_bin_file(const char *bin, uint8_t **data, off_t *bin_size) { FILE *g; struct stat bin_stat; @@ -170,7 +170,8 @@ err:

• • 0 - on success
• • -1 - on failure

*/ -static int write_capsule_file(FILE *f, void *data, size_t size, const char *msg) +static int write_capsule_file(FILE *f, const void *data, size_t size,

Why should size not be constant?

The name of the parameter 'size' does not match the function documentation which has 'bin_size'. Please, change either of both.

Parameter 'f' does not match the documentation which has 'bin'.

For each function that you touch, please, ensure that the function parameters are correctly documented.

• 	      const char *msg)
  

  { size_t size_written;

@@ -343,7 +344,8 @@ static int create_auth_data(struct auth_context *ctx)

• • 0 - on success
• • -1 - on failure

*/ -static int dump_signature(const char *path, uint8_t *signature, size_t sig_size) +static int dump_signature(const char *path, const uint8_t *signature,

• 	  size_t sig_size)
  

Why should sig_size not be constant?

{ char *sig_path; FILE *f; @@ -402,10 +404,12 @@ static void free_sig_data(struct auth_context *ctx)

• • 0 - on success
• • -1 - on failure

*/ -static int create_fwbin(char *path, char *bin, efi_guid_t *guid,

• 	unsigned long index, unsigned long instance,
  

• 	struct fmp_payload_header_params *fmp_ph_params,
  

• 	uint64_t mcount, char *privkey_file, char *cert_file,
  

+static int create_fwbin(const char *path, const char *bin,

• 	const efi_guid_t *guid, unsigned long index,
  

• 	unsigned long instance,
  

• 	const struct fmp_payload_header_params *fmp_ph_params,
  

• 	uint64_t mcount,
  

• 	const char *privkey_file, const char *cert_file,
  uint16_t oemflags)
  

Why shouldn't instance, mcount, oemflags be constant?

{ struct efi_capsule_header header; @@ -604,7 +608,8 @@ void convert_uuid_to_guid(unsigned char *buf) buf[7] = c; }

-static int create_empty_capsule(char *path, efi_guid_t *guid, bool fw_accept) +static int create_empty_capsule(const char *path, const efi_guid_t *guid,

• 		bool fw_accept)
  

Why should fw_accept not be constant?

Please, make the use of 'const' a bit more consistent.

Best regards

Heinrich

{ struct efi_capsule_header header = { 0 }; FILE *f = NULL; @@ -666,7 +671,7 @@ int main(int argc, char **argv) unsigned long index, instance; uint64_t mcount; unsigned long oemflags;

• char *privkey_file, *cert_file;

• const char *privkey_file, *cert_file; int c, idx; struct fmp_payload_header_params fmp_ph_params = { 0 };