unofficial mirror of guix-patches@gnu.org 
 help / color / mirror / code / Atom feed
From: "Ludovic Courtès" <ludo@gnu.org>
To: Tobias Kortkamp <tobias.kortkamp@gmail.com>
Cc: 55541-done@debbugs.gnu.org
Subject: bug#55541: [PATCH] gnu: Add azpainter.
Date: Fri, 24 Jun 2022 22:56:06 +0200	[thread overview]
Message-ID: <87sfntu6ft.fsf@gnu.org> (raw)
In-Reply-To: <6d31ff958ec0c75cbba8324a275315d195a54902.1653045472.git.tobias.kortkamp@gmail.com> (Tobias Kortkamp's message of "Fri, 20 May 2022 13:18:39 +0200")

Hi Tobias,

Tobias Kortkamp <tobias.kortkamp@gmail.com> skribis:

> * gnu/packages/graphics.scm (azpainter): New variable.

Applied, thanks!

Maxime Devos <maximedevos@telenet.be> skribis:

> As-is, this home-grown build system is broken when cross-compiling:
>
>   * When cross-compiling, TARGET-gcc needs to be used instead of gcc.
>     Maybe do (setenv "CC" #$(cc-for-target)) first?
>
>   * Likewise, TARGET-pkg-config instead of pkg-config (not 100% sure)
>
>   * It tries to run binaries during ./configure.  When cross-compiling,
>     ./conftest will always fail (unless using emulation) and hence
>     always detect ‘little endian’ but this is incorrect when
>     cross-compiling for big-endian architectures.
>
> (Needs some fixes or work-arounds.)  You can test with "guix build
> azpainter --target=aarch64-linux-gnu" or such.
>
> Also, some other problems.  From mlk_studio.c
>
> int mFILEreadBE32(FILE *fp,void *buf)
> {
> 	uint8_t v[4];
>
> 	if(fread(v, 1, 4, fp) < 4)
> 		return 1;
> 	else
> 	{
> 		*((uint32_t *)buf) = ((uint32_t)v[0] << 24) | (v[1] <<
> 16) | (v[2] << 8) | v[3];
> 		return 0;
> 	}
> }
>
> looks like a potential strict-aliasing violation to me, resulting in
> undefined behaviour -- what if buf is a pointer to an array of, say,
> doubles?  Also a potential alignment problem, though maybe it's only
> called for sufficiently aligned 'buf'.  The strict-aliasing problem
> can be worked around with -fno-strict-aliasing or maybe just -fno-ipa-
> strict-aliasing , though I don't know if that's sufficient.

These are all good points and I appreciate that you did such a thorough
review (audit?) of the package!

That said, I think it’s a bit too much to ask of a downstream packager
or user to address these issues.  As I see it, these issues should be
reported upstream and addressed upstream.

I hope that makes sense!

Thanks,
Ludo’.




  parent reply	other threads:[~2022-06-24 20:57 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-20 11:18 [bug#55541] [PATCH] gnu: Add azpainter Tobias Kortkamp
2022-05-20 16:00 ` Maxime Devos
2022-06-24 20:56 ` Ludovic Courtès [this message]
2022-06-24 21:41   ` Maxime Devos

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://guix.gnu.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87sfntu6ft.fsf@gnu.org \
    --to=ludo@gnu.org \
    --cc=55541-done@debbugs.gnu.org \
    --cc=tobias.kortkamp@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

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).