Hello Daniel,
On 10/15/19 4:19 PM, Daniel Kiper wrote:
> On Sat, Oct 05, 2019 at 12:44:25AM +0200, Javier Martinez Canillas wrote:
>> From: Paulo Flabiano Smorigo <pfsmor...@br.ibm.com>
>>
>> This patch sets a net_<interface>_clientid and net_<interface>_clientuuid
>> GRUB environment variables, using the DHCP client ID and UUID options if
>> these are found.
>>
>> In the same way than net_<interface>_<option> variables are set for other
>> options such domain name, boot file, next server, etc.
>>
>> Signed-off-by: Paulo Flabiano Smorigo <pfsmor...@br.ibm.com>
>> Signed-off-by: Javier Martinez Canillas <javi...@redhat.com>
>
> In general "Reviewed-by: Daniel Kiper <daniel.ki...@oracle.com>"
> but a few nit picks below...
>
Thanks for the review.
>> ---
>>
>> grub-core/net/bootp.c | 85 +++++++++++++++++++++++++++++++++++++++----
>> include/grub/net.h | 2 +
>> 2 files changed, 79 insertions(+), 8 deletions(-)
>>
>> diff --git a/grub-core/net/bootp.c b/grub-core/net/bootp.c
>> index 04cfbb04504..0e6e41a1699 100644
>> --- a/grub-core/net/bootp.c
>> +++ b/grub-core/net/bootp.c
>> @@ -95,6 +95,49 @@ enum
>> /* Max timeout when waiting for BOOTP/DHCP reply */
>> #define GRUB_DHCP_MAX_PACKET_TIMEOUT 32
>>
>> +static char *
>> +grub_env_write_readonly (struct grub_env_var *var __attribute__ ((unused)),
>> + const char *val __attribute__ ((unused)))
>
> s/grub_env_write_readonly/grub_env_ro_write/?
>
I noticed that this function already exists in grub-core/net/net.c, take a
look to commit e4dbf247b65 ("add grub_env_set_net_property function").
So I'll just remove this and the cryptic set_env_limn_ro() function.
>> +{
>> + return NULL;
>> +}
>> +
>> +static void
>> +set_env_limn_ro (const char *intername, const char *suffix,
>> + const char *value, grub_size_t len)
>
> I have hard time reading this. What does "limn" stand for?
> Could we make the name of this function less cryptic?
>
Yes, same. But this function also already exists in grub-core/net/net.c as
mentioned. It's called grub_env_set_net_property() and I'll use that.
Sorry for missing that this patch could be much more simpler after a rebase
to the latest GRUB version.
Best regards,
--
Javier Martinez Canillas
Software Engineer - Desktop Hardware Enablement
Red Hat
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
