diff mbox series

[bug#51747] : gnu: nix: Update to 2.4.

Message ID 86bl0da6rn.fsf@163.com
State Accepted
Headers show
Series [bug#51747] : gnu: nix: Update to 2.4. | expand

Checks

Context Check Description
cbaines/applying patch fail View Laminar job
cbaines/issue success View issue

Commit Message

Zhu Zihao Jan. 15, 2022, 5:47 a.m. UTC 
Do some modification based on your code, and add your credit.

Comments

M Jan. 15, 2022, 10:52 a.m. UTC | #1 
Hi,

I looked a bit in the source code, and it seems to be missing
some error checking.  E.g., in libcpuid/cpuid_main.c:,
it is not verified that 'malloc' succeeded.

        list->names = (char**) malloc(sizeof(char*) * n);
        [...]
        list->names[n] = (char*) malloc (i - last)
        [....]
        list->names[n][i - last - 1 = '\0']

It doesn't seem exploitable here, but I would recommend something
like 'xmalloc' instead to avoid (anti-)learning by bad examples.

Also, in the package definition, you are using cmake-build-system.
This works, but the README doesn't mention CMake anywhere and instead
mentions the autotools (autoconf etc.) and has configure.ac, so I would
use gnu-build-system instead.

Zhu Zihao schreef op za 15-01-2022 om 13:47 [+0800]:
> +      (arguments
> +       (list
> +        #:configure-flags #~(list "-DLIBCPUID_TESTS=ON")
> +        #:phases
> +        #~(modify-phases %standard-phases
> +            (add-after 'unpack 'absolutize
> +              (lambda* (#:key inputs #:allow-other-keys)
> +                ;; Linux specific
> +                (when #$(target-linux?)
> +                  (substitute* "libcpuid/rdmsr.c"
> +                    (("modprobe") (which "modprobe")))))))))

This use of 'which' doesn't work when cross-compiling because it looks
in $PATH (which contains (possibly implicit) native-inputs) and not
'inputs' -- 'inputs' and 'native-inputs' are merged together when
compiling natively so this doesn't cause errors when compiling natively
but it doesn't work when cross-compiling -- try
"./pre-inst-env guix build libcpuid --target=aarch64-linux-gnu".

I would use 'search-input-file' instead:

        ;; using (when #$(target-linux? ...) ...) instead
        ;; of 'guard' would also work
        #~(modify-phases %standard-phases
            (add-after 'unpack 'absolutize
              (lambda* (#:key inputs #:allow-other-keys)
                ;; modprobe doesn't exist on the Hurd 
                (guard (c ((search-error? c) (values)))
                  (substitute* "libcpuid/rdmsr.c"
                    (("modprobe")
                     (search-input-file inputs "bin/modprobe")))))))))


> + confused with the @code{cpuid} command line utility from package
> + @code{cpuid}.")

A definite article is missing before 'package'.

> +      (native-inputs (list python-3))   ;required by tests

I would keep the original comment

      ;; a python from the 3. series is required by tests

here, because it is important that it comes from the 3. series
-- the shebang uses "/usr/bin/env python3", not "python3".  This
comment would be helpful when updating python to the 4. series
(these versions don't exist yet but presumably they eventually will),
as it would make it clear that we cannot simply replace 'python-3' with
'python-4'.

Greetings,
Maxime.
Zhu Zihao Jan. 15, 2022, 4:06 p.m. UTC | #2 
Maxime Devos <maximedevos@telenet.be> writes:

Please forgive me that I don't have so much time to test cross build. If
you have problem with my patches, you can modify it based on existing
patches. And don't forget to notice Guix committers to ensure they don't
miss any new patches. :)
Oleg Pykhalov Jan. 21, 2022, 2:29 p.m. UTC | #3 
Zhu Zihao <all_but_last@163.com> writes:

> Maxime Devos <maximedevos@telenet.be> writes:
>
> Please forgive me that I don't have so much time to test cross build. If
> you have problem with my patches, you can modify it based on existing
> patches. And don't forget to notice Guix committers to ensure they don't
> miss any new patches. :)

Pushed to master.

Closing the issue.

Thanks,
Oleg.
M Jan. 23, 2022, 11:02 a.m. UTC | #4 
Maxime Devos schreef op za 15-01-2022 om 11:52 [+0100]:
> I looked a bit in the source code, and it seems to be missing
> some error checking.  E.g., in libcpuid/cpuid_main.c:,
> it is not verified that 'malloc' succeeded.
> 
>         list->names = (char**) malloc(sizeof(char*) * n);
>         [...]
>         list->names[n] = (char*) malloc (i - last)
>         [....]
>         list->names[n][i - last - 1 = '\0']

This has been addressed upstream:
<https://github.com/anrieff/libcpuid/pull/160>.

Greetings,
Maxime.
diff mbox series

Patch

From a8ff6088e543f5a7322180aa59e7c4d5447eb11c Mon Sep 17 00:00:00 2001
From: Zhu Zihao <all_but_last@163.com>
Date: Fri, 14 Jan 2022 17:30:22 +0800
Subject: [PATCH 6/6] gnu: nix: Update to 2.5.1.

* gnu/packages/package-management.scm(nix): Update to 2.5.1.
---
 gnu/packages/package-management.scm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gnu/packages/package-management.scm b/gnu/packages/package-management.scm
index 594d6e9d4b..9882f0a411 100644
--- a/gnu/packages/package-management.scm
+++ b/gnu/packages/package-management.scm
@@ -662,7 +662,7 @@  (define-public guix-icons
 (define-public nix
   (package
     (name "nix")
-    (version "2.4")
+    (version "2.5.1")
     (source
      (origin
        (method git-fetch)
@@ -671,7 +671,7 @@  (define-public nix
              (commit version)))
        (file-name (git-file-name name version))
        (sha256
-        (base32 "1abgfw5ndqklm0x533li32l4azifz3f6lhaxm6s74b704043r7m2"))
+        (base32 "1m8rmv8i6lg83pmalvjlq1fn8mcghn3ngjv3kw1kqsa45ymj5sqq"))
        (patches
         (search-patches "nix-dont-build-html-doc.diff"))))
     (build-system gnu-build-system)
-- 
2.34.0