All of lore.kernel.org
 help / color / mirror / Atom feed
From: Anthony Foiani <tkil@scrye.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Randy Dunlap <rdunlap@infradead.org>, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] sysfs: add sysfs_create/remove_groups()
Date: Wed, 21 Aug 2013 22:55:19 -0600	[thread overview]
Message-ID: <g38q2l3yg.fsf@dworkin.scrye.com> (raw)
In-Reply-To: <20130821204750.GB4546@kroah.com> (Greg Kroah-Hartman's message of "Wed\, 21 Aug 2013 13\:47\:50 -0700")

Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:

> + * sysfs_create_groups - given a directory kobject, create a bunch of attribute groups
> + * @kobj:	The kobject to create the group on
> + * @groups:	The attribute groups to create, NULL terminated
> + *
> + * This function creates a bunch of attribute groups.  If an error occurs when
> + * creating a group, all previously created groups will be removed, unwinding
> + * everything back to the original state when this function was called.
> + * It will explicitly warn and error if any of the attribute files being
> + * created already exist.
> + *
> + * Returns 0 on success or error code from sysfs_create_groups on error.

"... from sysfs_create_group on error"
                       ^^^^^ (note singular, not plural)

Otherwise it's a bit of a tautology...

Maybe rename to "sysfs_(create|remove)_multiple_groups" (or
..._many_groups) to avoid this kind of one-char difference?

> + * sysfs_remove_groups - remove a list of groups
> + *
> + * kobj:	The kobject for the groups to be removed from
> + * groups:	NULL terminated list of groups to be removed
> + *
> + * If groups is not NULL, the all groups will be removed from the kobject

"If groups is not NULL, remove listed groups from the kobject"

(I also played around with using pointer arithmetic instead of array
accesses for these two functions; it cut a few lines of code, but not
enough to bother, I don't think.)

Thanks,
Tony

  reply	other threads:[~2013-08-22  5:03 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-08-21 20:35 [PATCH] sysfs: add sysfs_create/remove_groups() Greg Kroah-Hartman
2013-08-21 20:38 ` Randy Dunlap
2013-08-21 20:44   ` Greg Kroah-Hartman
2013-08-21 20:47   ` Greg Kroah-Hartman
2013-08-22  4:55     ` Anthony Foiani [this message]
2013-08-22 16:22       ` Greg Kroah-Hartman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=g38q2l3yg.fsf@dworkin.scrye.com \
    --to=tkil@scrye.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rdunlap@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.