On Wed, Feb 08, 2017 at 02:28:55PM +0300, Konstantin Khlebnikov wrote: > Null kernfs nodes could be found at cgroups during construction.
Really? Does this happen today? Is this an issue for older kernels as well? > It seems safer to handle these null pointers right in kernfs in > the same way as printf prints "(null)" for null pointer string. > > Signed-off-by: Konstantin Khlebnikov <[email protected]> > Acked-by: Tejun Heo <[email protected]> > --- > fs/kernfs/dir.c | 10 ++++++++++ > include/trace/events/cgroup.h | 20 ++++++-------------- > 2 files changed, 16 insertions(+), 14 deletions(-) > > diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c > index cf4c636ff4da..439b946c4808 100644 > --- a/fs/kernfs/dir.c > +++ b/fs/kernfs/dir.c > @@ -41,6 +41,9 @@ static bool kernfs_lockdep(struct kernfs_node *kn) > > static int kernfs_name_locked(struct kernfs_node *kn, char *buf, size_t > buflen) > { > + if (!kn) > + return strlcpy(buf, "(null)", buflen); Why not return an error? > + > return strlcpy(buf, kn->parent ? kn->name : "/", buflen); > } > > @@ -110,6 +113,8 @@ static struct kernfs_node *kernfs_common_ancestor(struct > kernfs_node *a, > * kn_to: /n1/n2/n3 [depth=3] > * result: /../.. > * > + * [3] when @kn_to is NULL result will be "(null)" > + * > * Returns the length of the full path. If the full length is equal to or > * greater than @buflen, @buf contains the truncated path with the trailing > * '\0'. On error, -errno is returned. > @@ -123,6 +128,9 @@ static int kernfs_path_from_node_locked(struct > kernfs_node *kn_to, > size_t depth_from, depth_to, len = 0; > int i, j; > > + if (!kn_to) > + return strlcpy(buf, "(null)", buflen); > + > if (!kn_from) > kn_from = kernfs_root(kn_to)->kn; > > @@ -166,6 +174,8 @@ static int kernfs_path_from_node_locked(struct > kernfs_node *kn_to, > * similar to strlcpy(). It returns the length of @kn's name and if @buf > * isn't long enough, it's filled upto @buflen-1 and nul terminated. > * > + * Fills buffer with "(null)" if @kn is NULL. > + * > * This function can be called from any context. > */ > int kernfs_name(struct kernfs_node *kn, char *buf, size_t buflen) > diff --git a/include/trace/events/cgroup.h b/include/trace/events/cgroup.h > index ab68640a18d0..c226f50e88fa 100644 > --- a/include/trace/events/cgroup.h > +++ b/include/trace/events/cgroup.h > @@ -61,19 +61,15 @@ DECLARE_EVENT_CLASS(cgroup, > __field( int, id ) > __field( int, level ) > __dynamic_array(char, path, > - cgrp->kn ? cgroup_path(cgrp, NULL, 0) + 1 > - : strlen("(null)")) > + cgroup_path(cgrp, NULL, 0) + 1) Ah, you are trying to make this "simpler", is that the case? So this is just a "cleanup", not a bugfix? thanks, greg k-h
