! Please note that this is a snapshot of our old Bugzilla server, which is read only since May 29, 2020. Please go to gitlab.xfce.org for our new server !
different integer types for brightness level on (ac|battery)
Status:
RESOLVED: FIXED
Product:
Xfce4-power-manager
Component:
General

Comments

Description Klaus Flittner 2012-05-06 17:47:17 CEST
Created attachment 4400 
fix dim on inactivity

The property for brightness level on ac|battery is defined as g(u)int. But in xfpm-backlight.c the value is read to an uninitialized glong. 

On amd64 these type have different sizes (32bit vs. 64bit). Under certain conditions the high bits aren't zero and so the brightness level is read wrong.

Attached is a patch that fixes this by initializing the variable dim_level to zero.
Comment 1 Yves-Alexis Perez editbugs 2012-07-18 09:51:44 CEST
Hmhm, I'm a bit puzzled about this, in modern compilers, shouldn't variable be initialized at 0 anyway?
Comment 2 Klaus Flittner 2012-07-19 20:48:19 CEST
According to the C standard only static variables are initialized to zero. All normal variables have an undefined value.
If in both usages of the value the same type was used then an initialisation wouldn't be necessary. But writing an gint to the memory of glong variable only overwrites the lower 32bits in this case and the high 32bit remain unchanged.
So eihter the types have to be the same or the varibale needs to be initialized.
Comment 3 Yves-Alexis Perez editbugs 2012-07-20 00:27:49 CEST
(In reply to comment #2)
> So eihter the types have to be the same or the varibale needs to be
> initialized.

I think the former is more consistent (and consistent with other fixes of the same issues in other properties).
Comment 4 Brandon Watkins 2012-10-17 20:22:01 CEST
I can confirm that this patch fixes this bug for me on xfce 4.10 (xubuntu 12.10). This bug was causing the power manager to never dim my screen on battery, after applying the patch everything works as expected.

When will this be fixed upstream?
Comment 5 Bob Scott 2012-12-08 22:41:59 CET
Will this be fixed in the next version of XFCE? as it is currently xfce power manager is rather useless on 64-bit because of this problem, the screen dimming stuff just doesn't work at all.
Comment 6 Massimo Cavalleri 2013-02-16 15:36:38 CET
Created attachment 4920 
unify level brightness to gint

This patch unify level's brightness to gint.
With this previous patch that set dim_level = 0 is not more useful.
Comment 7 Christoph Junghans 2013-06-19 07:13:04 CEST
*** Bug 9629 has been marked as a duplicate of this bug. ***
Comment 8 Felipe Contreras 2013-07-05 06:32:21 CEST
(In reply to Klaus Flittner from comment #0)
> Created attachment 4400 
> fix dim on inactivity
> 
> The property for brightness level on ac|battery is defined as g(u)int. But
> in xfpm-backlight.c the value is read to an uninitialized glong. 
> 
> On amd64 these type have different sizes (32bit vs. 64bit). Under certain
> conditions the high bits aren't zero and so the brightness level is read
> wrong.
> 
> Attached is a patch that fixes this by initializing the variable dim_level
> to zero.

Why is this patch not applied? It's obviously correct.
Comment 9 Yves-Alexis Perez editbugs 2013-07-05 07:24:24 CEST
(In reply to Felipe Contreras from comment #8)
> (In reply to Klaus Flittner from comment #0)
> > Created attachment 4400 
> > fix dim on inactivity
> > 
> > The property for brightness level on ac|battery is defined as g(u)int. But
> > in xfpm-backlight.c the value is read to an uninitialized glong. 
> > 
> > On amd64 these type have different sizes (32bit vs. 64bit). Under certain
> > conditions the high bits aren't zero and so the brightness level is read
> > wrong.
> > 
> > Attached is a patch that fixes this by initializing the variable dim_level
> > to zero.
> 
> Why is this patch not applied? It's obviously correct.

Because nobody takes care of xfpm currently?
Comment 10 Martin Matuska 2013-07-23 08:59:22 CEST
Created attachment 5098 
Change brightness level from glong to gint32

The "Backlight" RandR property is a 32-bit integer. This means that the int32 (gint32) type should be used to represent brightness levels. The attached patch does nothing else than changing the brightness level representation from glong to gint32. This fixes the screen auto-dimming issue and brightness panel plugin issue.
Comment 11 Nick Schermer editbugs 2013-07-23 11:28:27 CEST
Pushed patch

Bug #8840

Reported by:
Klaus Flittner
Reported on: 2012-05-06
Last modified on: 2013-07-23
Duplicates (1):
  • 9629 screen doesn't get dimmed automatically when unplugging the ac adapter

People

Assignee:
Ali Abdallah
CC List:
8 users

Version

Version:
Unspecified

Attachments

fix dim on inactivity (472 bytes, patch)
2012-05-06 17:47 CEST , Klaus Flittner
no flags
unify level brightness to gint (12.84 KB, patch)
2013-02-16 15:36 CET , Massimo Cavalleri
no flags
Change brightness level from glong to gint32 (9.22 KB, patch)
2013-07-23 08:59 CEST , Martin Matuska
no flags

Additional information