lkml.org 
[lkml]   [2008]   [Jun]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC][PATCH] configfs: Report errors in config_*_init_type_name()
On Wed, Jun 18, 2008 at 08:30:51PM +0200, Louis Rilling wrote:
> [ applies on top of http://lkml.org/lkml/2008/6/12/427 ]
>
> config_item_set_name() may fail but its error code is not checked in
> config_*_init_type_name().
>
> This patch adds the missing error checking and make config_*_init_type_name()
> report errors. In-tree users are updated to report errors as well.

While this patch is correct on the face, I'd like to try a
different approach. I wasn't thinking about it right.
See, config_*_init_type_name() are generally a create-time thing.
Almost everyone uses it without error checking because they know it is
safe; they are usually using a static name. config_item_set_name()
can only error if strlen(name)>CONFIGFS_ITEM_NAME_LEN. That's why
config_*_init_type_name() are void.
In other words, we shouldn't be adding useless error-check
boilerplate for already-safe things.
But there are a couple of users of config_*_set_type_name() that
aren't safe. The lockspace in fs/dlm/config.c is one (lockspace names
can be 64 characters). The config_*_init_type_name() helpers are quite
convenient.
I see two choices:

1) Make your changes to return errors from config_*_init_type_name(),
but don't check the errors on known-safe usage (small static
strings).
2) Provide two API, one that is void and one that is not, so that
known-safe usage can use the void call (and BUG_ON() if the strlen()
is off), while other usage checks the errors.

Joel

--

Life's Little Instruction Book #3

"Watch a sunrise at least once a year."

Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker@oracle.com
Phone: (650) 506-8127


\
 
 \ /
  Last update: 2008-06-18 22:25    [W:0.030 / U:0.212 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site