* [PATCH 1/2] Linux: implement getloadavg(3) using sysinfo(2)
@ 2021-08-06 19:17 Cristian Rodríguez
2021-08-06 19:17 ` [PATCH 2/2] resolv: make res_randomid use random_bits() Cristian Rodríguez
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Cristian Rodríguez @ 2021-08-06 19:17 UTC (permalink / raw)
To: libc-alpha
Signed-off-by: Cristian Rodríguez <crrodriguez@opensuse.org>
---
sysdeps/unix/sysv/linux/getloadavg.c | 50 ++++++++--------------------
1 file changed, 14 insertions(+), 36 deletions(-)
diff --git a/sysdeps/unix/sysv/linux/getloadavg.c b/sysdeps/unix/sysv/linux/getloadavg.c
index e50cc396e7..3ea7fd02b7 100644
--- a/sysdeps/unix/sysv/linux/getloadavg.c
+++ b/sysdeps/unix/sysv/linux/getloadavg.c
@@ -1,4 +1,4 @@
-/* Get system load averages. Linux (/proc/loadavg) version.
+/* Get system load averages. Linux version.
Copyright (C) 1999-2021 Free Software Foundation, Inc.
This file is part of the GNU C Library.
@@ -16,53 +16,31 @@
License along with the GNU C Library; if not, see
<https://www.gnu.org/licenses/>. */
-#include <errno.h>
-#include <fcntl.h>
-#include <locale.h>
-#include <stdlib.h>
-#include <unistd.h>
-#include <not-cancel.h>
+#include <array_length.h>
+#include <sys/param.h>
+#include <sys/sysinfo.h>
/* Put the 1 minute, 5 minute and 15 minute load averages
into the first NELEM elements of LOADAVG.
Return the number written (never more than 3, but may be less than NELEM),
or -1 if an error occurred. */
+#define CLAMP(v, lo, hi) MIN (MAX (v, lo), hi)
+
+#define SYSINFO_LOADS_SCALE (1 << SI_LOAD_SHIFT)
+
int
getloadavg (double loadavg[], int nelem)
{
- int fd;
+ struct sysinfo info = {};
- fd = __open_nocancel ("/proc/loadavg", O_RDONLY);
- if (fd < 0)
+ if (__sysinfo (&info) != 0)
return -1;
- else
- {
- char buf[65], *p;
- ssize_t nread;
- int i;
- nread = __read_nocancel (fd, buf, sizeof buf - 1);
- __close_nocancel_nostatus (fd);
- if (nread <= 0)
- return -1;
- buf[nread - 1] = '\0';
+ nelem = CLAMP (nelem, 0, (int)array_length (info.loads));
- if (nelem > 3)
- nelem = 3;
- p = buf;
- for (i = 0; i < nelem; ++i)
- {
- char *endp;
- loadavg[i] = __strtod_l (p, &endp, _nl_C_locobj_ptr);
- if (endp == p)
- /* This should not happen. The format of /proc/loadavg
- must have changed. Don't return with what we have,
- signal an error. */
- return -1;
- p = endp;
- }
+ for (int i = 0; i < nelem; i++)
+ loadavg[i] = (double)info.loads[i] / SYSINFO_LOADS_SCALE;
- return i;
- }
+ return nelem;
}
--
2.32.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/2] resolv: make res_randomid use random_bits()
2021-08-06 19:17 [PATCH 1/2] Linux: implement getloadavg(3) using sysinfo(2) Cristian Rodríguez
@ 2021-08-06 19:17 ` Cristian Rodríguez
2021-08-12 18:51 ` Cristian Rodríguez
2021-08-20 7:00 ` Florian Weimer
2021-08-12 18:51 ` [PATCH 1/2] Linux: implement getloadavg(3) using sysinfo(2) Cristian Rodríguez
2021-08-19 19:17 ` Adhemerval Zanella
2 siblings, 2 replies; 9+ messages in thread
From: Cristian Rodríguez @ 2021-08-06 19:17 UTC (permalink / raw)
To: libc-alpha
It is at least "more random" than 0xffff & __getpid ();
Signed-off-by: Cristian Rodríguez <crrodriguez@opensuse.org>
---
resolv/res_randomid.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/resolv/res_randomid.c b/resolv/res_randomid.c
index 546daf4c8b..08a52a111a 100644
--- a/resolv/res_randomid.c
+++ b/resolv/res_randomid.c
@@ -84,9 +84,10 @@
#include <resolv.h>
#include <unistd.h>
+#include <random-bits.h>
unsigned int
res_randomid (void) {
- return 0xffff & __getpid ();
+ return random_bits();
}
libc_hidden_def (__res_randomid)
--
2.32.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] resolv: make res_randomid use random_bits()
2021-08-06 19:17 ` [PATCH 2/2] resolv: make res_randomid use random_bits() Cristian Rodríguez
@ 2021-08-12 18:51 ` Cristian Rodríguez
2021-08-20 7:00 ` Florian Weimer
1 sibling, 0 replies; 9+ messages in thread
From: Cristian Rodríguez @ 2021-08-12 18:51 UTC (permalink / raw)
To: libc-alpha
Ping..
On Fri, Aug 6, 2021 at 3:18 PM Cristian Rodríguez
<crrodriguez@opensuse.org> wrote:
>
> It is at least "more random" than 0xffff & __getpid ();
>
> Signed-off-by: Cristian Rodríguez <crrodriguez@opensuse.org>
> ---
> resolv/res_randomid.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/resolv/res_randomid.c b/resolv/res_randomid.c
> index 546daf4c8b..08a52a111a 100644
> --- a/resolv/res_randomid.c
> +++ b/resolv/res_randomid.c
> @@ -84,9 +84,10 @@
>
> #include <resolv.h>
> #include <unistd.h>
> +#include <random-bits.h>
>
> unsigned int
> res_randomid (void) {
> - return 0xffff & __getpid ();
> + return random_bits();
> }
> libc_hidden_def (__res_randomid)
> --
> 2.32.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] resolv: make res_randomid use random_bits()
2021-08-06 19:17 ` [PATCH 2/2] resolv: make res_randomid use random_bits() Cristian Rodríguez
2021-08-12 18:51 ` Cristian Rodríguez
@ 2021-08-20 7:00 ` Florian Weimer
2021-08-20 13:23 ` Cristian Rodríguez
1 sibling, 1 reply; 9+ messages in thread
From: Florian Weimer @ 2021-08-20 7:00 UTC (permalink / raw)
To: Cristian Rodríguez; +Cc: libc-alpha
* Cristian Rodríguez:
> It is at least "more random" than 0xffff & __getpid ();
>
> Signed-off-by: Cristian Rodríguez <crrodriguez@opensuse.org>
> ---
> resolv/res_randomid.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/resolv/res_randomid.c b/resolv/res_randomid.c
> index 546daf4c8b..08a52a111a 100644
> --- a/resolv/res_randomid.c
> +++ b/resolv/res_randomid.c
> @@ -84,9 +84,10 @@
>
> #include <resolv.h>
> #include <unistd.h>
> +#include <random-bits.h>
>
> unsigned int
> res_randomid (void) {
> - return 0xffff & __getpid ();
> + return random_bits();
> }
> libc_hidden_def (__res_randomid)
The masking with 0xffff needs to remain, and there should be a space
before ().
Thanks,
Florian
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] resolv: make res_randomid use random_bits()
2021-08-20 7:00 ` Florian Weimer
@ 2021-08-20 13:23 ` Cristian Rodríguez
0 siblings, 0 replies; 9+ messages in thread
From: Cristian Rodríguez @ 2021-08-20 13:23 UTC (permalink / raw)
To: Florian Weimer; +Cc: libc-alpha
On Fri, Aug 20, 2021 at 3:00 AM Florian Weimer <fweimer@redhat.com> wrote:
> > libc_hidden_def (__res_randomid)
>
> The masking with 0xffff needs to remain,
arrgh..damn ! id is an unsigned short..
you are right.
and there should be a space
> before ().
Ok thanks..
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] Linux: implement getloadavg(3) using sysinfo(2)
2021-08-06 19:17 [PATCH 1/2] Linux: implement getloadavg(3) using sysinfo(2) Cristian Rodríguez
2021-08-06 19:17 ` [PATCH 2/2] resolv: make res_randomid use random_bits() Cristian Rodríguez
@ 2021-08-12 18:51 ` Cristian Rodríguez
2021-08-19 19:17 ` Adhemerval Zanella
2 siblings, 0 replies; 9+ messages in thread
From: Cristian Rodríguez @ 2021-08-12 18:51 UTC (permalink / raw)
To: libc-alpha
ping
On Fri, Aug 6, 2021 at 3:18 PM Cristian Rodríguez
<crrodriguez@opensuse.org> wrote:
>
> Signed-off-by: Cristian Rodríguez <crrodriguez@opensuse.org>
> ---
> sysdeps/unix/sysv/linux/getloadavg.c | 50 ++++++++--------------------
> 1 file changed, 14 insertions(+), 36 deletions(-)
>
> diff --git a/sysdeps/unix/sysv/linux/getloadavg.c b/sysdeps/unix/sysv/linux/getloadavg.c
> index e50cc396e7..3ea7fd02b7 100644
> --- a/sysdeps/unix/sysv/linux/getloadavg.c
> +++ b/sysdeps/unix/sysv/linux/getloadavg.c
> @@ -1,4 +1,4 @@
> -/* Get system load averages. Linux (/proc/loadavg) version.
> +/* Get system load averages. Linux version.
> Copyright (C) 1999-2021 Free Software Foundation, Inc.
> This file is part of the GNU C Library.
>
> @@ -16,53 +16,31 @@
> License along with the GNU C Library; if not, see
> <https://www.gnu.org/licenses/>. */
>
> -#include <errno.h>
> -#include <fcntl.h>
> -#include <locale.h>
> -#include <stdlib.h>
> -#include <unistd.h>
> -#include <not-cancel.h>
> +#include <array_length.h>
> +#include <sys/param.h>
> +#include <sys/sysinfo.h>
>
> /* Put the 1 minute, 5 minute and 15 minute load averages
> into the first NELEM elements of LOADAVG.
> Return the number written (never more than 3, but may be less than NELEM),
> or -1 if an error occurred. */
>
> +#define CLAMP(v, lo, hi) MIN (MAX (v, lo), hi)
> +
> +#define SYSINFO_LOADS_SCALE (1 << SI_LOAD_SHIFT)
> +
> int
> getloadavg (double loadavg[], int nelem)
> {
> - int fd;
> + struct sysinfo info = {};
>
> - fd = __open_nocancel ("/proc/loadavg", O_RDONLY);
> - if (fd < 0)
> + if (__sysinfo (&info) != 0)
> return -1;
> - else
> - {
> - char buf[65], *p;
> - ssize_t nread;
> - int i;
>
> - nread = __read_nocancel (fd, buf, sizeof buf - 1);
> - __close_nocancel_nostatus (fd);
> - if (nread <= 0)
> - return -1;
> - buf[nread - 1] = '\0';
> + nelem = CLAMP (nelem, 0, (int)array_length (info.loads));
>
> - if (nelem > 3)
> - nelem = 3;
> - p = buf;
> - for (i = 0; i < nelem; ++i)
> - {
> - char *endp;
> - loadavg[i] = __strtod_l (p, &endp, _nl_C_locobj_ptr);
> - if (endp == p)
> - /* This should not happen. The format of /proc/loadavg
> - must have changed. Don't return with what we have,
> - signal an error. */
> - return -1;
> - p = endp;
> - }
> + for (int i = 0; i < nelem; i++)
> + loadavg[i] = (double)info.loads[i] / SYSINFO_LOADS_SCALE;
>
> - return i;
> - }
> + return nelem;
> }
> --
> 2.32.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] Linux: implement getloadavg(3) using sysinfo(2)
2021-08-06 19:17 [PATCH 1/2] Linux: implement getloadavg(3) using sysinfo(2) Cristian Rodríguez
2021-08-06 19:17 ` [PATCH 2/2] resolv: make res_randomid use random_bits() Cristian Rodríguez
2021-08-12 18:51 ` [PATCH 1/2] Linux: implement getloadavg(3) using sysinfo(2) Cristian Rodríguez
@ 2021-08-19 19:17 ` Adhemerval Zanella
2021-08-19 21:38 ` Cristian Rodríguez
2 siblings, 1 reply; 9+ messages in thread
From: Adhemerval Zanella @ 2021-08-19 19:17 UTC (permalink / raw)
To: libc-alpha, Cristian Rodríguez
On 06/08/2021 16:17, Cristian Rodríguez wrote:
> Signed-off-by: Cristian Rodríguez <crrodriguez@opensuse.org>
Look good, some minor comments below.
Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
> ---
> sysdeps/unix/sysv/linux/getloadavg.c | 50 ++++++++--------------------
> 1 file changed, 14 insertions(+), 36 deletions(-)
>
> diff --git a/sysdeps/unix/sysv/linux/getloadavg.c b/sysdeps/unix/sysv/linux/getloadavg.c
> index e50cc396e7..3ea7fd02b7 100644
> --- a/sysdeps/unix/sysv/linux/getloadavg.c
> +++ b/sysdeps/unix/sysv/linux/getloadavg.c
> @@ -1,4 +1,4 @@
> -/* Get system load averages. Linux (/proc/loadavg) version.
> +/* Get system load averages. Linux version.
> Copyright (C) 1999-2021 Free Software Foundation, Inc.
> This file is part of the GNU C Library.
>
Ok.
> @@ -16,53 +16,31 @@
> License along with the GNU C Library; if not, see
> <https://www.gnu.org/licenses/>. */
>
> -#include <errno.h>
> -#include <fcntl.h>
> -#include <locale.h>
> -#include <stdlib.h>
> -#include <unistd.h>
> -#include <not-cancel.h>
> +#include <array_length.h>
> +#include <sys/param.h>
> +#include <sys/sysinfo.h>
>
> /* Put the 1 minute, 5 minute and 15 minute load averages
> into the first NELEM elements of LOADAVG.
> Return the number written (never more than 3, but may be less than NELEM),
> or -1 if an error occurred. */
>
> +#define CLAMP(v, lo, hi) MIN (MAX (v, lo), hi)
> +
> +#define SYSINFO_LOADS_SCALE (1 << SI_LOAD_SHIFT)
> +
> int
> getloadavg (double loadavg[], int nelem)
> {
> - int fd;
> + struct sysinfo info = {};
I think there is no need to clear the struct prior the syscall,
I would expect the kernel to fill all the appropriated values
(as it seems to be doing on kernel/sys.c).
>
> - fd = __open_nocancel ("/proc/loadavg", O_RDONLY);
> - if (fd < 0)
> + if (__sysinfo (&info) != 0)
> return -1;
> - else
> - {
> - char buf[65], *p;
> - ssize_t nread;
> - int i;
>
> - nread = __read_nocancel (fd, buf, sizeof buf - 1);
> - __close_nocancel_nostatus (fd);
> - if (nread <= 0)
> - return -1;
> - buf[nread - 1] = '\0';
> + nelem = CLAMP (nelem, 0, (int)array_length (info.loads));
I think there is no need to cast to int here.
>
> - if (nelem > 3)
> - nelem = 3;
> - p = buf;
> - for (i = 0; i < nelem; ++i)
> - {
> - char *endp;
> - loadavg[i] = __strtod_l (p, &endp, _nl_C_locobj_ptr);
> - if (endp == p)
> - /* This should not happen. The format of /proc/loadavg
> - must have changed. Don't return with what we have,
> - signal an error. */
> - return -1;
> - p = endp;
> - }
> + for (int i = 0; i < nelem; i++)
> + loadavg[i] = (double)info.loads[i] / SYSINFO_LOADS_SCALE;
>
> - return i;
> - }
> + return nelem;
> }
>
Ok.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] Linux: implement getloadavg(3) using sysinfo(2)
2021-08-19 19:17 ` Adhemerval Zanella
@ 2021-08-19 21:38 ` Cristian Rodríguez
2021-08-20 14:21 ` Adhemerval Zanella
0 siblings, 1 reply; 9+ messages in thread
From: Cristian Rodríguez @ 2021-08-19 21:38 UTC (permalink / raw)
To: Adhemerval Zanella; +Cc: libc-alpha
On Thu, Aug 19, 2021 at 3:17 PM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
>
>
> On 06/08/2021 16:17, Cristian Rodríguez wrote:
> > Signed-off-by: Cristian Rodríguez <crrodriguez@opensuse.org>
>
> Look good, some minor comments below.
First, thanks for looking..
> > + struct sysinfo info = {};
>
> I think there is no need to clear the struct prior the syscall,
> I would expect the kernel to fill all the appropriated values
> (as it seems to be doing on kernel/sys.c).
Yes, I read the kernel source, I just decided not to count on the
kernel doing it.
> > + nelem = CLAMP (nelem, 0, (int)array_length (info.loads));
>
> I think there is no need to cast to int here
IIRC the compiler complained I was comparing signed and unsigned
values.. array_length expands to a size_t value.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] Linux: implement getloadavg(3) using sysinfo(2)
2021-08-19 21:38 ` Cristian Rodríguez
@ 2021-08-20 14:21 ` Adhemerval Zanella
0 siblings, 0 replies; 9+ messages in thread
From: Adhemerval Zanella @ 2021-08-20 14:21 UTC (permalink / raw)
To: Cristian Rodríguez; +Cc: libc-alpha
On 19/08/2021 18:38, Cristian Rodríguez wrote:
> On Thu, Aug 19, 2021 at 3:17 PM Adhemerval Zanella
> <adhemerval.zanella@linaro.org> wrote:
>>
>>
>>
>> On 06/08/2021 16:17, Cristian Rodríguez wrote:
>>> Signed-off-by: Cristian Rodríguez <crrodriguez@opensuse.org>
>>
>> Look good, some minor comments below.
>
> First, thanks for looking..
>
>>> + struct sysinfo info = {};
>>
>> I think there is no need to clear the struct prior the syscall,
>> I would expect the kernel to fill all the appropriated values
>> (as it seems to be doing on kernel/sys.c).
>
>
> Yes, I read the kernel source, I just decided not to count on the
> kernel doing it.
In this case, clearing the struct does not really improve for the
hypothetical kernel that does not set all the bits because you
won't know if the returned value has meaningful data or not.
>
>
>>> + nelem = CLAMP (nelem, 0, (int)array_length (info.loads));
>>
>> I think there is no need to cast to int here
In this case I think it would be better to make CLAMP a proper
inline function that returns size_t.
>
> IIRC the compiler complained I was comparing signed and unsigned
> values.. array_length expands to a size_t value.
>
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-08-20 14:21 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-06 19:17 [PATCH 1/2] Linux: implement getloadavg(3) using sysinfo(2) Cristian Rodríguez
2021-08-06 19:17 ` [PATCH 2/2] resolv: make res_randomid use random_bits() Cristian Rodríguez
2021-08-12 18:51 ` Cristian Rodríguez
2021-08-20 7:00 ` Florian Weimer
2021-08-20 13:23 ` Cristian Rodríguez
2021-08-12 18:51 ` [PATCH 1/2] Linux: implement getloadavg(3) using sysinfo(2) Cristian Rodríguez
2021-08-19 19:17 ` Adhemerval Zanella
2021-08-19 21:38 ` Cristian Rodríguez
2021-08-20 14:21 ` Adhemerval Zanella
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).