Hello Heinrich,
thank you for you extensive review. I will incorporate your reviews in a future version of the patch series.
Best Regards Malte
Am 16.06.2023 um 20:18 schrieb Heinrich Schuchardt:
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 };