In comment #2 I wrote:
> Comment on attachment 68712 [details] [review]
>...
> Although you could make the penultimate line as follows I don't recommend it
> because it would hide any problems like the escaping issue that could cause
> HIBERNATE_MODE_SAVE to be invalid:
>
> [ -n "$HIBERNATE_MODE_SAVE" ] && echo -n "$HIBERNATE_MODE_SAVE" >
> /sys/power/disk
In fact there are 2 cases as the code is used now:
1 HIBERNATE_MODE unset => normal hibernate
2 HIBERNATE_MODE = suspend => suspend-hybrid
Given which, I've revised my comment above and propose a new version of the modified do_hibernate() as follows:
The key points:
- hibernate_mode_save is only set if the current HIBERNATE_MODE is being changed (which only happens, if it does, in the suspend-hybrid case);
- on resume the hibernate mode is only restored if hibernate_mode_save was set.
This fixes:
- the failure to restore the hibernate mode with suspend-hybrid;
- "sh: I/O error" on resume from suspend-hybrid
- "sh: I/O error" on resume from hibernate.
Finally, the pm_functions script uses "echo -n" (from line 318, including the above patch) and local declarations while the comment against function log() implies that the script is aiming for POSIX conformance and yet other functions use non-POSIX local declarations. local and "echo -n" usages are fine for Debian and derived environments. To achieve POSIX conformance such usages would have to be reviewed and modified; "echo -n" could be replaced with printf (the parameters to be echoed in each case being plain text not containing formatting commands); or a shell function echo() can be added based on log().
In comment #2 I wrote: MODE_SAVE" ] && echo -n "$HIBERNATE_ MODE_SAVE" >
> Comment on attachment 68712 [details] [review]
>...
> Although you could make the penultimate line as follows I don't recommend it
> because it would hide any problems like the escaping issue that could cause
> HIBERNATE_MODE_SAVE to be invalid:
>
> [ -n "$HIBERNATE_
> /sys/power/disk
In fact there are 2 cases as the code is used now:
1 HIBERNATE_MODE unset => normal hibernate
2 HIBERNATE_MODE = suspend => suspend-hybrid
Given which, I've revised my comment above and propose a new version of the modified do_hibernate() as follows:
do_hibernate()
{
local hibernate_mode_save ret
[ -n "${HIBERNATE_MODE}" ] && \ mode_save= $(cat /sys/power/disk) && \ mode_save= "${hibernate_ mode_save# #*\[}" && \ mode_save= "${hibernate_ mode_save% %\]*}" && \ mode_save" != "${HIBERNATE_MODE}" ] || \ mode_save= "" mode_save" ] && \ mode_save" ] && \ mode_save" > /sys/power/disk
grep -qw "${HIBERNATE_MODE}" /sys/power/disk && \
hibernate_
hibernate_
hibernate_
[ "$hibernate_
hibernate_
[ -n "$hibernate_
echo -n "${HIBERNATE_MODE}" > /sys/power/disk
echo -n "disk" > /sys/power/state
ret=$?
[ -n "$hibernate_
echo -n "$hibernate_
return $ret
}
The key points:
- hibernate_mode_save is only set if the current HIBERNATE_MODE is being changed (which only happens, if it does, in the suspend-hybrid case);
- on resume the hibernate mode is only restored if hibernate_mode_save was set.
This fixes:
- the failure to restore the hibernate mode with suspend-hybrid;
- "sh: I/O error" on resume from suspend-hybrid
- "sh: I/O error" on resume from hibernate.
Finally, the pm_functions script uses "echo -n" (from line 318, including the above patch) and local declarations while the comment against function log() implies that the script is aiming for POSIX conformance and yet other functions use non-POSIX local declarations. local and "echo -n" usages are fine for Debian and derived environments. To achieve POSIX conformance such usages would have to be reviewed and modified; "echo -n" could be replaced with printf (the parameters to be echoed in each case being plain text not containing formatting commands); or a shell function echo() can be added based on log().