Skip to contentSkip to conversation

prog/init/lm_sensors.init: reduce verbosity, MAKEDEV, usage

PATCH New Axel Thimm 3 participants 4 messages May 19, 2005

lore
Share & tools
lore
#1 lore raw

PATCH 1/1 prog/init/lm_sensors.init: reduce verbosity, MAKEDEV,

This patch has me thinking...

What do people think about changing the format of the sysconfig/lm_sensors config file from

MODULE_0=i2c-i801
MODULE_1=i2c-isa
MODULE_2=lm85
MODULE_3îprom
MODULE_4=w83781d

to

BUSES="i2c-i801 i2c-isa"
MODULES="lm85 eeprom w83781d"

If we did this, then we could get rid of the confusing and obscure backticks and eval in the module loading loop, replacing it with something simple like:

for module in $MODULES ; do

      echo -n __$module__
      modprobe $module

done

Which seems much clearer to me.

This would also mean that users would no longer have to edit variable names to keep them sequential when adding, removing or re-ordering modules.

If I hear agreement, I'll work up a patch. We'll need to modify sensors-detect to generate the new format.

:v)

Quoted 79 lines · Axel Thimm

Hi,

the attached patch is partly including some patching by Red Hat, which are distribution independent.

It exits early on absence of config files, or empty config files (no MODULE_* entries) to avoid noise at startup of lm_sensors unconfigured systems.

There is also a MAKEDEV in the start command. This may be required for proper udev operation.

It also fixes a couple of minor other things like to output of the usage ($0 instead of fixed "sensors.init" etc).

------------------------------------------------------------------------

Index: prog/init/lm_sensors.init
=================================>RCS file: /home/cvs/lm_sensors2/prog/init/lm_sensors.init,v retrieving revision 1.6
diff -u -d -r1.6 lm_sensors.init

init/lm_sensors.init +0 -0
@@ -47,6 +47,12 @@

fi

CONFIG=/etc/sysconfig/lm_sensors
+[ -r "$CONFIG" ] || exit 0

init/lm_sensors.init +5 -0
+egrep '^MODULE_' $CONFIG &>/dev/null || exit 0
+
+# Load config file
+. "$CONFIG"
+

PSENSORS=/usr/local/bin/sensors

if [ ! -x $PSENSORS ]; then

init/lm_sensors.init +0 -0
@@ -57,16 +63,17 @@

. /etc/init.d/functions

RETVAL=0
+prog="lm_sensors"

start() {

init/lm_sensors.init +2 -2
-	echo -n $"Starting up sensors: "
-	test -r "$CONFIG" && . "$CONFIG"
+	echo -n $"Starting $prog: starting module "
 
+	/sbin/MAKEDEV i2c

	modules=`grep \^MODULE_ $CONFIG | wc -l | tr -d ' '`
	i=0
	while [ $i -lt $modules ] ; do
		module=`eval echo '$'MODULE_$i`
init/lm_sensors.init +1 -1
-		echo starting module __${module}__
+		echo -n "__${module}__ "

		/sbin/modprobe $module &>/dev/null
		i=`expr $i + 1`
	done
init/lm_sensors.init +2 -3
@@ -83,8 +90,7 @@
 }
 
 stop() {
-	echo -n $"Shutting down sensors: "
-	test -r "$CONFIG" && . "$CONFIG"
+	echo -n $"Stopping $prog: "
 
 	modules=`grep \^MODULE_ $CONFIG | wc -l | tr -d ' '`
 	i=`expr $modules`

@@ -141,7 +147,7 @@
 	condrestart
 	;;
  *)
-	echo "Usage: sensors.init {start|stop|status|restart|reload|condrestart}"
+	echo $"Usage: $0 {start|stop|status|restart|reload|condrestart}"
 	exit 1

esac

Signature · 4 lines
Philip Pokorny, Director of Engineering
Tel: 415-954-2823   Fax: 415-954-2899   Toll Free: 888-PENGUIN
PENGUIN COMPUTING, INC.
www.penguincomputing.com
#2 lore raw

PATCH 1/1 prog/init/lm_sensors.init: reduce verbosity, MAKEDEV,

In reply to Axel Thimm

Hi Philip,

Quoted 11 lines

This patch has me thinking...

What do people think about changing the format of the sysconfig/lm_sensors config file from

MODULE_0=i2c-i801
MODULE_1=i2c-isa
MODULE_2=lm85
MODULE_3îprom
MODULE_4=w83781d

Quoted 4 lines

to

BUSES="i2c-i801 i2c-isa"
MODULES="lm85 eeprom w83781d"

The names you pick are confusing. Bus drivers are modules too. BUSES and CHIPS would be better.

Quoted 10 lines

If we did this, then we could get rid of the confusing and obscure backticks and eval in the module loading loop, replacing it with something simple like:

for module in $MODULES ; do

     echo -n __$module__
     modprobe $module

done

Which seems much clearer to me.

Agreed. However, I wonder why things were done that way in the first place. Were we following some Red Hat standard? I have no Red Hat-like system to verify that. If there is no standard to follow, any code simplification is welcome.

Thanks,

#3 lore raw

PATCH 1/1 prog/init/lm_sensors.init: reduce verbosity, MAKEDEV,

In reply to Axel Thimm

Hi Axel,

Quoted 6 lines

the attached patch is partly including some patching by Red Hat, which are distribution independent.

It exits early on absence of config files, or empty config files (no MODULE_* entries) to avoid noise at startup of lm_sensors unconfigured systems.

Sounds sane to me.

Quoted 2 lines

There is also a MAKEDEV in the start command. This may be required for proper udev operation.

Doesn't sound good to me. For one thing not all systems may have /sbin/MAKEDEV, for another what is fine for udev might not be fine for devfs or static /dev systems. Also, I thought that udev was totally dynamic so I don't understand why we would have to create the device files. Last, /dev/i2c* files are not needed for proper operation of the hardware monitoring modules, only for sensors-detect and dump tools.

Quoted 2 lines

It also fixes a couple of minor other things like to output of the usage ($0 instead of fixed "sensors.init" etc).

Sounds fine.

More comments inline:

CONFIG=/etc/sysconfig/lm_sensors
+[ -r "$CONFIG" ] || exit 0
+egrep '^MODULE_' $CONFIG &>/dev/null || exit 0

Why egrep and not simply grep? I'd also suggest -q instead of &>/dev/null. BTW, what's the difference between ">" and "&>"? The latter is not documented in my sh man page (">&" is).

- echo -n $"Starting up sensors: "
- test -r "$CONFIG" && . "$CONFIG"
+ echo -n $"Starting $prog: starting module "

What is the leading $ for, BTW?

From a technical point of view, we are not starting modules, but loading them.

- echo starting module __${module}__
+ echo -n "__${module}__ "

Do we need these extra underscores at all? Looks ugly IMHO.

Thanks,
--
Jean Delvare