On Tue, Mar 24, 2020 at 02:21:59PM -0700, Linus Torvalds wrote: > On Tue, Mar 24, 2020 at 1:46 PM Alexey Gladkov <gladkov.alexey@xxxxxxxxx> wrote: > > > > +/* definitions for hide_pid field */ > > +enum { > > + HIDEPID_OFF = 0, > > + HIDEPID_NO_ACCESS = 1, > > + HIDEPID_INVISIBLE = 2, > > +}; > > Should this enum be named... > > > struct proc_fs_info { > > struct pid_namespace *pid_ns; > > struct dentry *proc_self; /* For /proc/self */ > > struct dentry *proc_thread_self; /* For /proc/thread-self */ > > + kgid_t pid_gid; > > + int hide_pid; > > }; > > .. and then used here instead of "int"? > > Same goes for 'struct proc_fs_context' too, for that matter? > > And maybe in the function declarations and definitions too? In things > like 'has_pid_permissions()' (the series adds some other cases later, > like hidepid2str() etc) > > Yeah, enums and ints are kind of interchangeable in C, but even if it > wouldn't give us any more typechecking (except perhaps with sparse if > you mark it so), it would be documenting the use. > > Or am I missing something? > > Anyway, I continue to think the series looks fine, bnut would love to > see it in -next and perhaps comments from Al and Alexey Dobriyan.. Patches are OK, except the part where "pid" is named "pidfs" and the suffix doesn't convey any information. mount -t proc -o subset=pid,sysctl,misc Reviewed-by: Alexey Dobriyan <adobriyan@xxxxxxxxx>