Hi Martin, Martin Natano wrote on Sat, Jul 29, 2017 at 08:45:05PM +0200: > On Sat, Jul 29, 2017 at 02:19:50PM +0200, Martin Natano wrote: >> On Sat, Jul 29, 2017 at 02:03:22PM +0200, Mark Kettenis wrote:
>>> I don't think we want to add string parsing like this in the kernel. >>> Maybe the sysctl(8) frontend should do the mapping from strings to >>> numbers? >> Ok, I'll try to come up with an alternative diff that does the parsing >> in sysctl(8). Let me fetch my rubber gloves... > Here's the alternative diff. Ok? Here are my two cents: For users, "=hibernate" is easier to understand than "=2", so i sympathize with sysctl(8) taking the former. Users rarely have to use sysctl(3) directly, so i don't think it matters much whether the C interface takes a string or an int; do whatever is safer in the kernel. All the same, i'd like to point out that there is a minor documentation issue. Some time ago, we decided that having duplicate descriptions of each and every sysctl variable in sysctl(3) and sysctl(8) is bad for maintainability, deleted them all from sysctl(8), and made sure users of sysctl(8) can understand the full listing in sysctl(3) by adding the full string-names used by sysctl(8) to the sysctl(3) manual page. So after your patch, we would have to document in sysctl(3) that CTL_MACHDEP.CPU_LIDACTION = machdep.lidaction takes int on the C interface level and strings on the command line, and that 0=none, 1=suspend, 2=hibernate. While i don't know whether documenting *all* machdep variables in sysctl(3) would make sense, machdep.lidaction certainly seems important enough to document it. Same for machdep.kbdreset. Yours, Ingo
