> -----Original Message----- > From: Matt Porter [mailto:mporter@xxxxxxxxxx] > Sent: Thursday, April 03, 2014 3:22 PM > To: Krzysztof Opasiak > Cc: linux-usb@xxxxxxxxxxxxxxx; Andrzej Pietrasiewicz; Karol > Lewandowski; Stanislaw Wadas; ty317.kim@xxxxxxxxxxx; Marek > Szyprowski; Robert Baldyga > Subject: Re: [PATCH v2 5/5] libusbg: Use config ID and label > instead of config name. > > On Wed, Apr 02, 2014 at 07:42:42PM +0200, Krzysztof Opasiak wrote: > > Naming convention of Config FS should not be exposed > > to user of library. All API functions should use > > configuration ID (configuration number) as unique > > identificator and configuration label as human > > readable description. > > 100% agree, this looks mostly good but see below... > > > > > Signed-off-by: Krzysztof Opasiak <k.opasiak@xxxxxxxxxxx> > > --- > > examples/gadget-acm-ecm.c | 4 +- > > examples/show-gadgets.c | 9 +- > > include/usbg/usbg.h | 35 +++++--- > > src/usbg.c | 200 > +++++++++++++++++++++++++++++++-------------- > > 4 files changed, 168 insertions(+), 80 deletions(-) > > > > diff --git a/examples/gadget-acm-ecm.c b/examples/gadget-acm- > ecm.c > > index 8eb4aeb..7a8b71b 100644 > > --- a/examples/gadget-acm-ecm.c > > +++ b/examples/gadget-acm-ecm.c > > @@ -97,8 +97,8 @@ int main(void) > > goto out2; > > } > > > > - usbg_ret = usbg_create_config(g, "c.1", NULL /* use defaults > */, &c_strs, > > - &c); > > + usbg_ret = usbg_create_config(g, 1, "The only one", > > + NULL /* use defaults */, &c_strs, &c); > > This comment style is unusual, please don't use this going inline > with > the arguments. > > usbg_ret = usbg_create_config(g, 1, "The only one", > NULL, &c_strs, &c); > > if the NULL argument needs a description it can get something > self-documenting as a preprocessor symbol or put a comment before > this statement to make this example clear. And yes, clearly I > didn't > catch this on earlier patches. :) It's a example for new users of library so it should be well commented to help user understand what is each param for and what he can do with them. I think that it's quite popular convention to pass NULL if param but I have provided a comment to let user know that we also use this convention;) Comment style fixed and pull request updated. -- BR's Krzysiek -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
