[PATCH 4/5] libusbg: Replace usage of function name with type and instance.

2014-04-02 Thread Krzysztof Opasiak
User of library should not use directly function name but
only type of the function and name of instance. Using this
will separate user for naming convention on Config FS.

Signed-off-by: Krzysztof Opasiak k.opas...@samsung.com
---
 examples/show-gadgets.c |   20 ++--
 include/usbg/usbg.h |   29 --
 src/usbg.c  |  239 ++-
 3 files changed, 184 insertions(+), 104 deletions(-)

diff --git a/examples/show-gadgets.c b/examples/show-gadgets.c
index df86cb9..971beb4 100644
--- a/examples/show-gadgets.c
+++ b/examples/show-gadgets.c
@@ -70,11 +70,13 @@ void show_gadget(usbg_gadget *g)
 
 void show_function(usbg_function *f)
 {
-   char buf[USBG_MAX_STR_LENGTH];
+   char instance[USBG_MAX_STR_LENGTH];
+   usbg_function_type type;
int usbg_ret;
usbg_function_attrs f_attrs;
 
-   usbg_get_function_name(f, buf, USBG_MAX_STR_LENGTH);
+   usbg_get_function_instance(f, instance, USBG_MAX_STR_LENGTH);
+   type = usbg_get_function_type(f);
usbg_ret = usbg_get_function_attrs(f, f_attrs);
if (usbg_ret != USBG_SUCCESS) {
fprintf(stderr, Error: %s : %s\n, usbg_error_name(usbg_ret),
@@ -82,8 +84,9 @@ void show_function(usbg_function *f)
return;
}
 
-   fprintf(stdout,   Function '%s'\n, buf);
-   switch (usbg_get_function_type(f)) {
+   fprintf(stdout,   Function, type: %s instance: %s\n,
+   usbg_get_function_type_str(type), instance);
+   switch (type) {
case F_SERIAL:
case F_ACM:
case F_OBEX:
@@ -114,7 +117,8 @@ void show_config(usbg_config *c)
 {
usbg_binding *b;
usbg_function *f;
-   char buf[USBG_MAX_STR_LENGTH], buf2[USBG_MAX_STR_LENGTH];
+   char buf[USBG_MAX_STR_LENGTH], instance[USBG_MAX_STR_LENGTH];
+   usbg_function_type type;
usbg_config_attrs c_attrs;
usbg_config_strs c_strs;
int usbg_ret;
@@ -144,8 +148,10 @@ void show_config(usbg_config *c)
usbg_for_each_binding(b, c) {
usbg_get_binding_name(b, buf, USBG_MAX_STR_LENGTH);
f = usbg_get_binding_target(b);
-   usbg_get_function_name(f, buf2, USBG_MAX_STR_LENGTH);
-   fprintf(stdout, %s - %s\n, buf, buf2);
+   usbg_get_function_instance(f, instance, USBG_MAX_STR_LENGTH);
+   type = usbg_get_function_type(f);
+   fprintf(stdout, %s - %s %s\n, buf,
+   usbg_get_function_type_str(type), instance);
}
 }
 
diff --git a/include/usbg/usbg.h b/include/usbg/usbg.h
index d416624..35b35f7 100644
--- a/include/usbg/usbg.h
+++ b/include/usbg/usbg.h
@@ -255,10 +255,12 @@ extern usbg_gadget *usbg_get_gadget(usbg_state *s, const 
char *name);
 /**
  * @brief Get a function by name
  * @param g Pointer to gadget
- * @param name Name of the function
+ * @param type Function type
+ * @param instance Instance of function
  * @return Pointer to function or NULL if a matching function isn't found
  */
-extern usbg_function *usbg_get_function(usbg_gadget *g, const char *name);
+extern usbg_function *usbg_get_function(usbg_gadget *g,
+   usbg_function_type type, const char *instance);
 
 /**
  * @brief Get a configuration by name
@@ -517,20 +519,27 @@ extern int usbg_create_function(usbg_gadget *g, 
usbg_function_type type,
char *instance, usbg_function_attrs *f_attrs, usbg_function 
**f);
 
 /**
- * @brief Get function name length
- * @param f Config which name length should be returned
+ * @brief Get function instance name length
+ * @param f function which name length should be returned
  * @return Length of name string or usbg_error if error occurred.
  */
-extern size_t usbg_get_function_name_len(usbg_function *f);
+extern size_t usbg_get_function_instance_len(usbg_function *f);
 
 /**
- * @brieg Get config name
+ * @brief Get function instance name
  * @param f Pointer to function
- * @param buf Buffer where name should be copied
+ * @param buf Buffer where instance name should be copied
  * @param len Length of given buffer
  * @return 0 on success or usbg_error if error occurred.
  */
-extern int usbg_get_function_name(usbg_function *f, char *buf, size_t len);
+extern int usbg_get_function_instance(usbg_function *f, char *buf, size_t len);
+
+/**
+ * @brief Get function type as a string
+ * @param type Function type
+ * @return String suitable for given function type
+ */
+extern const char *usbg_get_function_type_str(usbg_function_type type);
 
 /* USB configurations allocation and configuration */
 
@@ -706,8 +715,8 @@ extern int usbg_get_gadget_udc(usbg_gadget *g, char *buf, 
size_t len);
 /**
  * @brief Get type of given function
  * @param f Pointer to function
- * @return Type of function
- * @warning Pointer to function has to be valid.
+ * @return usbg_function_type (0 or above) or
+ *  usbg_error (below 0) if error occurred
  */
 extern 

RE: [PATCH 4/5] libusbg: Replace usage of function name with type and instance.

2014-04-02 Thread David Laight
From: Opasiak
 User of library should not use directly function name but
 only type of the function and name of instance. Using this
 will separate user for naming convention on Config FS.

Personally I'd have thought it much better to pass in a
string name rather than a function number.
Far less scope for stupid errors.

You probably still want to check that the name passed is one
of the expected names.

David



--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 4/5] libusbg: Replace usage of function name with type and instance.

2014-04-02 Thread Krzysztof Opasiak
 -Original Message-
 From: David Laight [mailto:david.lai...@aculab.com]
 Sent: Wednesday, April 02, 2014 3:09 PM
 To: Krzysztof Opasiak; mpor...@linaro.org; linux-
 u...@vger.kernel.org
 Cc: Andrzej Pietrasiewicz; Karol Lewandowski; Stanislaw Wadas;
 ty317@samsung.com; Marek Szyprowski; Robert Baldyga
 Subject: RE: [PATCH 4/5] libusbg: Replace usage of function name
 with type and instance.
 
 From: Opasiak
  User of library should not use directly function name but
  only type of the function and name of instance. Using this
  will separate user for naming convention on Config FS.
 
 Personally I'd have thought it much better to pass in a
 string name rather than a function number.
 Far less scope for stupid errors.
 
 You probably still want to check that the name passed is one
 of the expected names.
 

In my opinion both approach are considerable. Currently function type is
represented in library using usbg_function_type enum. This approach also
has some advantages, you can simply iterate over function types and
checking the correctness of parameter is far faster and simpler than
checking if provided string is one of correct function type. Moreover
operating on enum is easier for API user than operating on string, think
about copying, passing to function, memory allocation etc. Of course we
could provide array of function names and users could use cons char*
pointer but I don't see any advantages in using such pointer instead of
enum.

Moreover, I'm not a fan of writing few times the same string because
compiler will not notify you about typos. If you make a typo in enum
value name (and there is no other enum value with such name) you will
get compilation error. When you will use acm string and make a typo
amc you won't get any errors or even warnings.

BR's
Krzysiek




--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 4/5] libusbg: Replace usage of function name with type and instance.

2014-04-02 Thread Matt Porter
On Wed, Apr 02, 2014 at 03:34:55PM +0200, Krzysztof Opasiak wrote:
  -Original Message-
  From: David Laight [mailto:david.lai...@aculab.com]
  Sent: Wednesday, April 02, 2014 3:09 PM
  To: Krzysztof Opasiak; mpor...@linaro.org; linux-
  u...@vger.kernel.org
  Cc: Andrzej Pietrasiewicz; Karol Lewandowski; Stanislaw Wadas;
  ty317@samsung.com; Marek Szyprowski; Robert Baldyga
  Subject: RE: [PATCH 4/5] libusbg: Replace usage of function name
  with type and instance.
  
  From: Opasiak
   User of library should not use directly function name but
   only type of the function and name of instance. Using this
   will separate user for naming convention on Config FS.
  
  Personally I'd have thought it much better to pass in a
  string name rather than a function number.
  Far less scope for stupid errors.
  
  You probably still want to check that the name passed is one
  of the expected names.
  
 
 In my opinion both approach are considerable. Currently function type is
 represented in library using usbg_function_type enum. This approach also
 has some advantages, you can simply iterate over function types and
 checking the correctness of parameter is far faster and simpler than
 checking if provided string is one of correct function type. Moreover
 operating on enum is easier for API user than operating on string, think
 about copying, passing to function, memory allocation etc. Of course we
 could provide array of function names and users could use cons char*
 pointer but I don't see any advantages in using such pointer instead of
 enum.
 
 Moreover, I'm not a fan of writing few times the same string because
 compiler will not notify you about typos. If you make a typo in enum
 value name (and there is no other enum value with such name) you will
 get compilation error. When you will use acm string and make a typo
 amc you won't get any errors or even warnings.

This was the whole point of having the enumed function types in the
first place, allow the compiler to help us find errors. So yes, I agree,
don't go down the path of passing in a string.

There's other subtle string naming requirements in the gadget configfs
API that ultimately we should hide. Like the requirement on the gadget
instance string or the syntax for configuration instances, for example.

-Matt
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html