On Wed, Jun 18, 2008 at 08:30:51PM +0200, Louis Rilling wrote:
quoted text
>  [ 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
--
unsubscribe notice
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/