[PATCH 1/5] mkeficapsule: constify function parameters

[Heinrich Schuchardt]

On 6/16/23 13:34, Stefan Herbrechtsmeier wrote:
> From: Malte Schmidt <malte.schmidt at 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 at weidmueller.com>
> Signed-off-by: Stefan Herbrechtsmeier <stefan.herbrechtsmeier at 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 };
>