* [PATCH] net: Don't use nested functions to allow building with clang
@ 2020-05-19 8:34 Javier Martinez Canillas
2020-05-19 12:27 ` Leif Lindholm
2020-05-19 14:35 ` Daniel Axtens
0 siblings, 2 replies; 4+ messages in thread
From: Javier Martinez Canillas @ 2020-05-19 8:34 UTC (permalink / raw)
To: grub-devel; +Cc: Javier Martinez Canillas, Daniel Axtens, Daniel Kiper
Nested functions are supported as an extension in GNU C, but are not in
the clang compiler. Commit cb2f15c5448 ("normal/main: Search for specific
config files for netboot") added a nested function which caused the build
to break when compiling with clang.
Reported-by: Daniel Axtens <dja@axtens.net>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
---
grub-core/net/net.c | 65 +++++++++++++++++++++++----------------------
1 file changed, 33 insertions(+), 32 deletions(-)
diff --git a/grub-core/net/net.c b/grub-core/net/net.c
index c42f0f4f71d..ec7e2899ed5 100644
--- a/grub-core/net/net.c
+++ b/grub-core/net/net.c
@@ -1735,42 +1735,43 @@ grub_net_restore_hw (void)
return GRUB_ERR_NONE;
}
-grub_err_t
-grub_net_search_config_file (char *config)
+static int grub_config_search_through (char *config, char *suffix,
+ grub_size_t num_tries,
+ grub_size_t slice_size)
{
- grub_size_t config_len;
- char *suffix;
+ while (num_tries-- > 0)
+ {
+ grub_file_t file;
- auto int search_through (grub_size_t num_tries, grub_size_t slice_size);
- int search_through (grub_size_t num_tries, grub_size_t slice_size)
- {
- while (num_tries-- > 0)
- {
- grub_file_t file;
+ grub_dprintf ("net", "attempt to fetch config %s\n", config);
- grub_dprintf ("net", "attempt to fetch config %s\n", config);
+ file = grub_file_open (config, GRUB_FILE_TYPE_CONFIG);
- file = grub_file_open (config, GRUB_FILE_TYPE_CONFIG);
+ if (file)
+ {
+ grub_file_close (file);
+ return 0;
+ }
+ else
+ {
+ if (grub_errno == GRUB_ERR_IO)
+ grub_errno = GRUB_ERR_NONE;
+ }
- if (file)
- {
- grub_file_close (file);
- return 0;
- }
- else
- {
- if (grub_errno == GRUB_ERR_IO)
- grub_errno = GRUB_ERR_NONE;
- }
+ if (grub_strlen (suffix) < slice_size)
+ break;
- if (grub_strlen (suffix) < slice_size)
- break;
+ config[grub_strlen (config) - slice_size] = '\0';
+ }
- config[grub_strlen (config) - slice_size] = '\0';
- }
+ return 1;
+}
- return 1;
- }
+grub_err_t
+grub_net_search_config_file (char *config)
+{
+ grub_size_t config_len;
+ char *suffix;
config_len = grub_strlen (config);
config[config_len] = '-';
@@ -1801,7 +1802,7 @@ grub_net_search_config_file (char *config)
if (client_uuid)
{
grub_strcpy (suffix, client_uuid);
- if (search_through (1, 0) == 0)
+ if (grub_config_search_through (config, suffix, 1, 0) == 0)
return GRUB_ERR_NONE;
}
@@ -1816,7 +1817,7 @@ grub_net_search_config_file (char *config)
if (*ptr == ':')
*ptr = '-';
- if (search_through (1, 0) == 0)
+ if (grub_config_search_through (config, suffix, 1, 0) == 0)
return GRUB_ERR_NONE;
/* By IP address */
@@ -1831,7 +1832,7 @@ grub_net_search_config_file (char *config)
((n >> 24) & 0xff), ((n >> 16) & 0xff), \
((n >> 8) & 0xff), ((n >> 0) & 0xff));
- if (search_through (8, 1) == 0)
+ if (grub_config_search_through (config, suffix, 8, 1) == 0)
return GRUB_ERR_NONE;
break;
}
@@ -1848,7 +1849,7 @@ grub_net_search_config_file (char *config)
*ptr = '-';
grub_snprintf (suffix, GRUB_NET_MAX_STR_ADDR_LEN, "%s", buf);
- if (search_through (1, 0) == 0)
+ if (grub_config_search_through (config, suffix, 1, 0) == 0)
return GRUB_ERR_NONE;
break;
}
--
2.26.2
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] net: Don't use nested functions to allow building with clang
2020-05-19 8:34 [PATCH] net: Don't use nested functions to allow building with clang Javier Martinez Canillas
@ 2020-05-19 12:27 ` Leif Lindholm
2020-05-19 14:35 ` Daniel Axtens
1 sibling, 0 replies; 4+ messages in thread
From: Leif Lindholm @ 2020-05-19 12:27 UTC (permalink / raw)
To: The development of GNU GRUB
Cc: Javier Martinez Canillas, Daniel Axtens, Daniel Kiper
Hi Javier,
On Tue, May 19, 2020 at 10:34:22 +0200, Javier Martinez Canillas wrote:
> Nested functions are supported as an extension in GNU C, but are not in
> the clang compiler. Commit cb2f15c5448 ("normal/main: Search for specific
> config files for netboot") added a nested function which caused the build
> to break when compiling with clang.
Thanks for this. And apologies for bikeshedding (but since there is a
history with GRUB and nested functions, I will) - could we reword this
to make fewer references to clang? Nested functions are an abomination
not supported by the C language. I.e. something like.
---
net: break out nested function
Nested functions are not supported in C, but are permitted as an
extension in the GNU C dialect. Commit cb2f15c5448 ("normal/main:
Search for specific config files for netboot") added a nested function
which caused the build to break when compiling with clang.
Break that out into a static helper function to make the code portable
again.
---
> Reported-by: Daniel Axtens <dja@axtens.net>
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
> ---
>
> grub-core/net/net.c | 65 +++++++++++++++++++++++----------------------
> 1 file changed, 33 insertions(+), 32 deletions(-)
>
> diff --git a/grub-core/net/net.c b/grub-core/net/net.c
> index c42f0f4f71d..ec7e2899ed5 100644
> --- a/grub-core/net/net.c
> +++ b/grub-core/net/net.c
> @@ -1735,42 +1735,43 @@ grub_net_restore_hw (void)
> return GRUB_ERR_NONE;
> }
>
> -grub_err_t
> -grub_net_search_config_file (char *config)
> +static int grub_config_search_through (char *config, char *suffix,
Style-wise, the function name should start in the first column on its
own line. (Running the file through gnu indent with default options
usually does the correct thing.)
/
Leif
> + grub_size_t num_tries,
> + grub_size_t slice_size)
> {
> - grub_size_t config_len;
> - char *suffix;
> + while (num_tries-- > 0)
> + {
> + grub_file_t file;
>
> - auto int search_through (grub_size_t num_tries, grub_size_t slice_size);
> - int search_through (grub_size_t num_tries, grub_size_t slice_size)
> - {
> - while (num_tries-- > 0)
> - {
> - grub_file_t file;
> + grub_dprintf ("net", "attempt to fetch config %s\n", config);
>
> - grub_dprintf ("net", "attempt to fetch config %s\n", config);
> + file = grub_file_open (config, GRUB_FILE_TYPE_CONFIG);
>
> - file = grub_file_open (config, GRUB_FILE_TYPE_CONFIG);
> + if (file)
> + {
> + grub_file_close (file);
> + return 0;
> + }
> + else
> + {
> + if (grub_errno == GRUB_ERR_IO)
> + grub_errno = GRUB_ERR_NONE;
> + }
>
> - if (file)
> - {
> - grub_file_close (file);
> - return 0;
> - }
> - else
> - {
> - if (grub_errno == GRUB_ERR_IO)
> - grub_errno = GRUB_ERR_NONE;
> - }
> + if (grub_strlen (suffix) < slice_size)
> + break;
>
> - if (grub_strlen (suffix) < slice_size)
> - break;
> + config[grub_strlen (config) - slice_size] = '\0';
> + }
>
> - config[grub_strlen (config) - slice_size] = '\0';
> - }
> + return 1;
> +}
>
> - return 1;
> - }
> +grub_err_t
> +grub_net_search_config_file (char *config)
> +{
> + grub_size_t config_len;
> + char *suffix;
>
> config_len = grub_strlen (config);
> config[config_len] = '-';
> @@ -1801,7 +1802,7 @@ grub_net_search_config_file (char *config)
> if (client_uuid)
> {
> grub_strcpy (suffix, client_uuid);
> - if (search_through (1, 0) == 0)
> + if (grub_config_search_through (config, suffix, 1, 0) == 0)
> return GRUB_ERR_NONE;
> }
>
> @@ -1816,7 +1817,7 @@ grub_net_search_config_file (char *config)
> if (*ptr == ':')
> *ptr = '-';
>
> - if (search_through (1, 0) == 0)
> + if (grub_config_search_through (config, suffix, 1, 0) == 0)
> return GRUB_ERR_NONE;
>
> /* By IP address */
> @@ -1831,7 +1832,7 @@ grub_net_search_config_file (char *config)
> ((n >> 24) & 0xff), ((n >> 16) & 0xff), \
> ((n >> 8) & 0xff), ((n >> 0) & 0xff));
>
> - if (search_through (8, 1) == 0)
> + if (grub_config_search_through (config, suffix, 8, 1) == 0)
> return GRUB_ERR_NONE;
> break;
> }
> @@ -1848,7 +1849,7 @@ grub_net_search_config_file (char *config)
> *ptr = '-';
>
> grub_snprintf (suffix, GRUB_NET_MAX_STR_ADDR_LEN, "%s", buf);
> - if (search_through (1, 0) == 0)
> + if (grub_config_search_through (config, suffix, 1, 0) == 0)
> return GRUB_ERR_NONE;
> break;
> }
> --
> 2.26.2
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] net: Don't use nested functions to allow building with clang
2020-05-19 8:34 [PATCH] net: Don't use nested functions to allow building with clang Javier Martinez Canillas
2020-05-19 12:27 ` Leif Lindholm
@ 2020-05-19 14:35 ` Daniel Axtens
2020-05-20 8:04 ` Daniel Axtens
1 sibling, 1 reply; 4+ messages in thread
From: Daniel Axtens @ 2020-05-19 14:35 UTC (permalink / raw)
To: Javier Martinez Canillas, grub-devel
Cc: Javier Martinez Canillas, Daniel Kiper
Hi Javier,
Thanks!
> Nested functions are supported as an extension in GNU C, but are not in
> the clang compiler. Commit cb2f15c5448 ("normal/main: Search for specific
> config files for netboot") added a nested function which caused the build
> to break when compiling with clang.
>
> Reported-by: Daniel Axtens <dja@axtens.net>
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
With this patch, clang-9 builds and runs mainline grub-emu.
Tested-by: Daniel Axtens <dja@axtens.net> # emu platform only
clang still doesn't like 'make check', but that looks like a different
issue around relocations. clang isn't a huge priority for me, so it
might take a while. But this patch is good to go in regardless.
Thanks again,
Daniel
> ---
>
> grub-core/net/net.c | 65 +++++++++++++++++++++++----------------------
> 1 file changed, 33 insertions(+), 32 deletions(-)
>
> diff --git a/grub-core/net/net.c b/grub-core/net/net.c
> index c42f0f4f71d..ec7e2899ed5 100644
> --- a/grub-core/net/net.c
> +++ b/grub-core/net/net.c
> @@ -1735,42 +1735,43 @@ grub_net_restore_hw (void)
> return GRUB_ERR_NONE;
> }
>
> -grub_err_t
> -grub_net_search_config_file (char *config)
> +static int grub_config_search_through (char *config, char *suffix,
> + grub_size_t num_tries,
> + grub_size_t slice_size)
> {
> - grub_size_t config_len;
> - char *suffix;
> + while (num_tries-- > 0)
> + {
> + grub_file_t file;
>
> - auto int search_through (grub_size_t num_tries, grub_size_t slice_size);
> - int search_through (grub_size_t num_tries, grub_size_t slice_size)
> - {
> - while (num_tries-- > 0)
> - {
> - grub_file_t file;
> + grub_dprintf ("net", "attempt to fetch config %s\n", config);
>
> - grub_dprintf ("net", "attempt to fetch config %s\n", config);
> + file = grub_file_open (config, GRUB_FILE_TYPE_CONFIG);
>
> - file = grub_file_open (config, GRUB_FILE_TYPE_CONFIG);
> + if (file)
> + {
> + grub_file_close (file);
> + return 0;
> + }
> + else
> + {
> + if (grub_errno == GRUB_ERR_IO)
> + grub_errno = GRUB_ERR_NONE;
> + }
>
> - if (file)
> - {
> - grub_file_close (file);
> - return 0;
> - }
> - else
> - {
> - if (grub_errno == GRUB_ERR_IO)
> - grub_errno = GRUB_ERR_NONE;
> - }
> + if (grub_strlen (suffix) < slice_size)
> + break;
>
> - if (grub_strlen (suffix) < slice_size)
> - break;
> + config[grub_strlen (config) - slice_size] = '\0';
> + }
>
> - config[grub_strlen (config) - slice_size] = '\0';
> - }
> + return 1;
> +}
>
> - return 1;
> - }
> +grub_err_t
> +grub_net_search_config_file (char *config)
> +{
> + grub_size_t config_len;
> + char *suffix;
>
> config_len = grub_strlen (config);
> config[config_len] = '-';
> @@ -1801,7 +1802,7 @@ grub_net_search_config_file (char *config)
> if (client_uuid)
> {
> grub_strcpy (suffix, client_uuid);
> - if (search_through (1, 0) == 0)
> + if (grub_config_search_through (config, suffix, 1, 0) == 0)
> return GRUB_ERR_NONE;
> }
>
> @@ -1816,7 +1817,7 @@ grub_net_search_config_file (char *config)
> if (*ptr == ':')
> *ptr = '-';
>
> - if (search_through (1, 0) == 0)
> + if (grub_config_search_through (config, suffix, 1, 0) == 0)
> return GRUB_ERR_NONE;
>
> /* By IP address */
> @@ -1831,7 +1832,7 @@ grub_net_search_config_file (char *config)
> ((n >> 24) & 0xff), ((n >> 16) & 0xff), \
> ((n >> 8) & 0xff), ((n >> 0) & 0xff));
>
> - if (search_through (8, 1) == 0)
> + if (grub_config_search_through (config, suffix, 8, 1) == 0)
> return GRUB_ERR_NONE;
> break;
> }
> @@ -1848,7 +1849,7 @@ grub_net_search_config_file (char *config)
> *ptr = '-';
>
> grub_snprintf (suffix, GRUB_NET_MAX_STR_ADDR_LEN, "%s", buf);
> - if (search_through (1, 0) == 0)
> + if (grub_config_search_through (config, suffix, 1, 0) == 0)
> return GRUB_ERR_NONE;
> break;
> }
> --
> 2.26.2
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] net: Don't use nested functions to allow building with clang
2020-05-19 14:35 ` Daniel Axtens
@ 2020-05-20 8:04 ` Daniel Axtens
0 siblings, 0 replies; 4+ messages in thread
From: Daniel Axtens @ 2020-05-20 8:04 UTC (permalink / raw)
To: Javier Martinez Canillas, grub-devel
Cc: Javier Martinez Canillas, Daniel Kiper
> clang still doesn't like 'make check', but that looks like a different
> issue around relocations. clang isn't a huge priority for me, so it
> might take a while. But this patch is good to go in regardless.
Turns out I needed to specify CXX=clang++ as well.
Daniel
>
> Thanks again,
> Daniel
>
>> ---
>>
>> grub-core/net/net.c | 65 +++++++++++++++++++++++----------------------
>> 1 file changed, 33 insertions(+), 32 deletions(-)
>>
>> diff --git a/grub-core/net/net.c b/grub-core/net/net.c
>> index c42f0f4f71d..ec7e2899ed5 100644
>> --- a/grub-core/net/net.c
>> +++ b/grub-core/net/net.c
>> @@ -1735,42 +1735,43 @@ grub_net_restore_hw (void)
>> return GRUB_ERR_NONE;
>> }
>>
>> -grub_err_t
>> -grub_net_search_config_file (char *config)
>> +static int grub_config_search_through (char *config, char *suffix,
>> + grub_size_t num_tries,
>> + grub_size_t slice_size)
>> {
>> - grub_size_t config_len;
>> - char *suffix;
>> + while (num_tries-- > 0)
>> + {
>> + grub_file_t file;
>>
>> - auto int search_through (grub_size_t num_tries, grub_size_t slice_size);
>> - int search_through (grub_size_t num_tries, grub_size_t slice_size)
>> - {
>> - while (num_tries-- > 0)
>> - {
>> - grub_file_t file;
>> + grub_dprintf ("net", "attempt to fetch config %s\n", config);
>>
>> - grub_dprintf ("net", "attempt to fetch config %s\n", config);
>> + file = grub_file_open (config, GRUB_FILE_TYPE_CONFIG);
>>
>> - file = grub_file_open (config, GRUB_FILE_TYPE_CONFIG);
>> + if (file)
>> + {
>> + grub_file_close (file);
>> + return 0;
>> + }
>> + else
>> + {
>> + if (grub_errno == GRUB_ERR_IO)
>> + grub_errno = GRUB_ERR_NONE;
>> + }
>>
>> - if (file)
>> - {
>> - grub_file_close (file);
>> - return 0;
>> - }
>> - else
>> - {
>> - if (grub_errno == GRUB_ERR_IO)
>> - grub_errno = GRUB_ERR_NONE;
>> - }
>> + if (grub_strlen (suffix) < slice_size)
>> + break;
>>
>> - if (grub_strlen (suffix) < slice_size)
>> - break;
>> + config[grub_strlen (config) - slice_size] = '\0';
>> + }
>>
>> - config[grub_strlen (config) - slice_size] = '\0';
>> - }
>> + return 1;
>> +}
>>
>> - return 1;
>> - }
>> +grub_err_t
>> +grub_net_search_config_file (char *config)
>> +{
>> + grub_size_t config_len;
>> + char *suffix;
>>
>> config_len = grub_strlen (config);
>> config[config_len] = '-';
>> @@ -1801,7 +1802,7 @@ grub_net_search_config_file (char *config)
>> if (client_uuid)
>> {
>> grub_strcpy (suffix, client_uuid);
>> - if (search_through (1, 0) == 0)
>> + if (grub_config_search_through (config, suffix, 1, 0) == 0)
>> return GRUB_ERR_NONE;
>> }
>>
>> @@ -1816,7 +1817,7 @@ grub_net_search_config_file (char *config)
>> if (*ptr == ':')
>> *ptr = '-';
>>
>> - if (search_through (1, 0) == 0)
>> + if (grub_config_search_through (config, suffix, 1, 0) == 0)
>> return GRUB_ERR_NONE;
>>
>> /* By IP address */
>> @@ -1831,7 +1832,7 @@ grub_net_search_config_file (char *config)
>> ((n >> 24) & 0xff), ((n >> 16) & 0xff), \
>> ((n >> 8) & 0xff), ((n >> 0) & 0xff));
>>
>> - if (search_through (8, 1) == 0)
>> + if (grub_config_search_through (config, suffix, 8, 1) == 0)
>> return GRUB_ERR_NONE;
>> break;
>> }
>> @@ -1848,7 +1849,7 @@ grub_net_search_config_file (char *config)
>> *ptr = '-';
>>
>> grub_snprintf (suffix, GRUB_NET_MAX_STR_ADDR_LEN, "%s", buf);
>> - if (search_through (1, 0) == 0)
>> + if (grub_config_search_through (config, suffix, 1, 0) == 0)
>> return GRUB_ERR_NONE;
>> break;
>> }
>> --
>> 2.26.2
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-05-20 8:04 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-05-19 8:34 [PATCH] net: Don't use nested functions to allow building with clang Javier Martinez Canillas
2020-05-19 12:27 ` Leif Lindholm
2020-05-19 14:35 ` Daniel Axtens
2020-05-20 8:04 ` Daniel Axtens
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.