[PATCH 1/5] mkeficapsule: constify function parameters

[Schmidt, Malte]

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 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 };
>>
>