Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: Build failure with newer gcc #14

Open
r-value opened this issue Oct 3, 2021 · 8 comments
Open

Bug: Build failure with newer gcc #14

r-value opened this issue Oct 3, 2021 · 8 comments

Comments

@r-value
Copy link

r-value commented Oct 3, 2021

slim/panel.cpp

Line 51 in 4a40caf

if (WinGC < 0) {

It triggered an error: ordered comparison of pointer with integer zero.

WinGC is a pointer and I can't figure out what WinGC < 0 means. This branch is never taken and actually optimized out if built with older gcc. Could you please explain or fix it?

@ncmprhnsbl
Copy link

@r-value
Copy link
Author

r-value commented Jan 18, 2022

@ncmprhnsbl
The patch works but obviously it made a functional change. I think it'll be better to confirm the meaning of this if branch with upstream vendor so I opened this issue.

@RobPearce
Copy link

I don't think that patch does "make a functional change" - it looks like it's simply correcting a long-standing mis-reading of the return value in case of error. As that error should never occur (well, not in normal use, anyway) it's a safe and correct patch.
I've applied it to my fork project at https://sourceforge.net/projects/slim-fork/

@maitra
Copy link

maitra commented Feb 25, 2023

I don't think that patch does "make a functional change" - it looks like it's simply correcting a long-standing mis-reading of the return value in case of error. As that error should never occur (well, not in normal use, anyway) it's a safe and correct patch. I've applied it to my fork project at https://sourceforge.net/projects/slim-fork/

The patch is also included in the Fedora package (which I am trying to unretire and modernise). There are also some other patches for fedora v1.3.3 and v1.3.2 for selinux. Do we still need these patches?

Sourceforge is a very irritating site to submit bugs, the captcha does not get loaded and so I gave up there. Perhaps you could have a github site for reporting bugs for slim-fork? Thanks!

@RobPearce
Copy link

There are also some other patches for fedora v1.3.3 and v1.3.2 for selinux. Do we still need these patches?

If you're referring to the slim-1.3.2-selinux.patch and slim-1.3.3-fedora.patch then both of them look to be specialisation of the configuration. If you intend to retain those Fedora-specific customisations then yes, you will need to apply updated versions of those two. The libpng fix has already been applied to the fork.

@maitra
Copy link

maitra commented Feb 25, 2023

There are also some other patches for fedora v1.3.3 and v1.3.2 for selinux. Do we still need these patches?

If you're referring to the slim-1.3.2-selinux.patch and slim-1.3.3-fedora.patch then both of them look to be specialisation of the configuration. If you intend to retain those Fedora-specific customisations then yes, you will need to apply updated versions of those two. The libpng fix has already been applied to the fork.

Thank you. Unfortunately, the patches seem to give errors with your build.

Btw, one more question: I would like to put the current fedora wallpaper in. How do I do that? Where do I specify it in the default configuration? Thank you.

@RobPearce
Copy link

RobPearce commented Feb 25, 2023

Unfortunately, the patches seem to give errors with your build.

They will need updating. The default / example configuration file has been updated and re-arranged, so those old patches won't apply as is. They aren't that big, so the best option may be to hand-edit a copy of the new file (copy & paste the changes, probably) and create a new patch with diff.

As to the wallpaper, you will need to create a "Fedora theme" with the background and panel you desire. There is some documentation on the project homepage. Then patch the "current_theme" line in slim.conf to reference it.

BTW, I've added an "Issues" tab on my github fork if you really can't get SourceForge to work.

@maitra
Copy link

maitra commented Feb 25, 2023

Thanks, the main differences seem to be rearrangement of the lines, and also uncommenting the daemon mode as well as the lockfile that used to be /var/log/slim.lock and is now /run/slim.pid.

thanks for the issues tab.

mbakke pushed a commit to guix-mirror/guix that referenced this issue Apr 30, 2023
GCC-11 sniffed out a long-standing bug where a pointer was being tested for a
negative value, which is impossible. Instead, check for NULL, which is how the
error result is actually returned.

See iwamatsu/slim#14 for details.

Fixes <https://issues.guix.gnu.org/63155>.

* gnu/packages/display-managers.scm (slim) [fix-0-pointer-comparison]: new
phase

Signed-off-by: Ludovic Courtès <ludo@gnu.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants