! 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 !
hddtemp support for xfce4-sensors-0.10.99.3 broken
Status:
RESOLVED: FIXED
Product:
Xfce4-sensors-plugin
Component:
General

Comments

Description Samuli Suominen 2007-05-23 08:20:44 CEST
hddtemp is installed in /usr/sbin/hddtemp in Gentoo, and I assume that'd be case for other distributions too. It's also installed without suidbit for security reasons. However, panel-plugin/sensors.c simply tries command hddtemp and expects it to be suidroot. Some suggestions..

- hddtemp binary location should be configurable from preferences.
- it should print a fat warning it's not suid root and thus it won't work.

but of course real solution..

- sensors should make use of hddtempd daemon which doesn't require root privileges and doesn't expose system to potential security race.
Comment 1 Samuli Suominen 2007-12-16 07:43:58 CET
Looks like 0.10.99.3 finds it from /usr/sbin now, but I don't think it's using the  sensorsd like it should yet.

Also the support is now entirely broken by:

'An error occured when executing /usr/sbin/hddtemp -F -n -q /dev/hda" /usr/sbin/hddtemp: invalid option -- F'

hddtemp --help:

unique xterm # hddtemp --help
 Usage: hddtemp [OPTIONS] [TYPE:]DISK1 [[TYPE:]DISK2]...

   hddtemp displays the temperature of drives supplied in argument.
   Drives must support S.M.A.R.T.

  TYPE could be SATA, PATA or SCSI. If omitted hddtemp will try to guess.

  -b   --drivebase   :  display database file content that allow hddtemp to
                        recognize supported drives.
  -D   --debug       :  display various S.M.A.R.T. fields and their values.
                        Useful to find a value that seems to match the
                        temperature and/or to send me a report.
                        (done for every drive supplied).
  -d   --daemon      :  run hddtemp in TCP/IP daemon mode (port 7634 by default.)
  -f   --file=FILE   :  specify database file to use.
  -l   --listen=addr :  listen on a specific interface (in TCP/IP daemon mode).
  -n   --numeric     :  print only the temperature.
  -p   --port=#      :  port to listen to (in TCP/IP daemon mode).
  -s   --separator=C :  separator to use between fields (in TCP/IP daemon mode).
  -S   --syslog=s    :  log temperature to syslog every s seconds.
  -u   --unit=[C|F]  :  force output temperature either in Celius or Fahrenheit.
  -q   --quiet       :  do not check if the drive is supported.
  -v   --version     :  display hddtemp version number.
  -w   --wake-up     :  wake-up the drive if need.
  -4                 :  listen on IPv4 sockets only.
  -6                 :  listen on IPv6 sockets only.

Report bugs or new drives to <hddtemp@guzu.net>.
hddtemp version 0.3-beta15

Note that 0.3-beta15 is latest upstream version, and been out a good while. It's also only version Gentoo ships.

And there is no /dev/hda on my system, it's /dev/sda.
Comment 2 Fabian Nowak editbugs 2007-12-16 10:12:34 CET
1. It is configurable and detected at configure time, and there's no "now" from /usr/sbin, because it has always been using it at least from /usr/sbin. I see no need to change its location at runtime, especially as hddtemp values are one feature of the plugin only, not aanything that should get its own boxes somewhere.
2. I tried to print the fat warning actually, the return codes are somehow changing over the releases, I think. Currently, I probably catch the wrong results and do only print the stdout. Gonna change that and print the fat warning in any case things went wrong.
3. I know of the netcat/nc solution for querying the daemon, but as its name depends on the OS, I didn't want to start using that yet. Seems like you would like it in 011.0 as well. Can probably search for nc/netcat at configure time.
4. On Debian, 0.3-beta15 and all the eralier releases have -F for foreground. It is not now completely broken, it always queried like that. Be careful with what you cry around. I am not gonna introduce #ifdef Gentoo: Do less and other. #endif
5. The plugin tries to get values for every disk in /sys/block (in linux 2.6, else from /proc/ide), so it also finds hda on your system, which is the fault of your system to show devices that are not installed. I don't have that Unfeature on Debian, only valid devices are showing up there. 
6. Whenever you raise or comment on bug reports, be careful with your tone, your accusation, your content. Prove carefully what you are writing. Ask instead of claiming things be wrong. The bug report's original text is perfect, the comments aren't.
Comment 3 Samuli Suominen 2007-12-16 10:24:08 CET
(In reply to comment #2)
> 4. On Debian, 0.3-beta15 and all the eralier releases have -F for foreground.
> It is not now completely broken, it always queried like that. Be careful with
> what you cry around. I am not gonna introduce #ifdef Gentoo: Do less and other.
> #endif

Gentoo is using upstream version which doesn't have -F, so Debian must be patching it with a patch not sent to upstream(?). 
The #ifdef could or should be for Debian only.

> 6. Whenever you raise or comment on bug reports, be careful with your tone,
> your accusation, your content. Prove carefully what you are writing. Ask
> instead of claiming things be wrong. The bug report's original text is perfect,
> the comments aren't.
> 

I didn't try to be offensive, I'm not a native speaker either. 
Sorry about that.
Comment 4 Fabian Nowak editbugs 2007-12-16 10:43:37 CET
ad 2. "Fixed": Same fat warning for both most probable error classes. More warning in case of other failures as well. Btw., it used to have a nice error code of 256 previously, but no longer. That's the reason.

ad 4. Removed -F. Seems to fairly work.

It's in the svn repository now. Thanks for your reports and provided details anyway.
Comment 5 Fabian Nowak editbugs 2008-06-27 18:48:17 CEST
Hddtemp support working in 0.10.99.5~svn again. Close it?
Comment 6 Samuli Suominen 2008-12-08 21:50:39 CET
I was sick and away for a while so sorry it took me so long to respond. Version .6 has a nice warning from libnotify which I got, and after adding suidbit to
hddtemp binary it was working as expected. So, closing as RESOLVED, FIXED.

Thanks again.

Bug #3270

Reported by:
Samuli Suominen
Reported on: 2007-05-23
Last modified on: 2009-07-15

People

Assignee:
Xfce-Goodies Maintainers
CC List:
1 user

Version

Version:
unspecified

Attachments

Additional information