sawmill_logger: don't try to access sysfs entries when not existent

[morphis]

Without this we have the following in the system log:

2012-11-19T08:10:34.479387Z [45097] tuna user.err sleepd: sysfs:SysfsGetDouble: Failed to
open file '/sys/devices/w1 bus master/w1_master_slaves/getrawcoulomb': No such file or
directory
2012-11-19T08:10:34.479875Z [45097] tuna user.err sleepd: sysfs:SysfsGetDouble: Failed to
open file '/sys/devices/w1 bus master/w1_master_slaves/getcoulomb': No such file or
directory

If the mentioned sysfs node is not available we return -1 to indicate there is nothing we can report.

regards,
Simon

[kuta42]

• *rc = -1;
• *c = -1;

Please write as -1.0 so that it's clear that the return value is a double.

If the mentioned sysfs node is not available we return -1 to indicate there is nothing we can report.

Does the caller recognize that -1.0 means "nothing we can report"?

[morphis]

Please write as -1.0 so that it's clear that the return value is double.

Done.

Does the caller recognize that -1.0 means "nothing we can report"?

The caller only prints the values to stdout and does nothing more with them. Reporting zero coulomb would let the user thing into the right direction when he see this in the log file.

[kuta42]

Ah, I see now that the caller is generating a log entry to be processed by our internal "sawmill" utility. My guess is that it's more likely to be able to handle values of 0.0 than values of -1.0, so how about returning the former?

Also, it seems wasteful to do the directory tests every time get_battery_coulomb_reading() called. These /sys directories aren't dynamic; if they aren't there the first time the routine is called, they'll never be there. A local static char* initialized to NULL could be used to store the directory to pass as the first argument to SysfsGetDouble() and be set the first time the routine is called; (char *)1 could be used to represent the case when neither of the /sys directories exist and the return values are to be set to 0.0 .

[kdopen]

Please don't use "special" pointer values. A pointer is either == NULL, or it's "valid". Just have a static bool or int to flag whether the directories exist. The same logic that would have set the pointer to (char *)1 can set the flag, and the intent is clearer.

[kuta42]

Agreed.

[morphis]

I updated the implementation according to the latest review comments.