[PATCH 4/5] libusbg: Replace usage of function name with type and instance.
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.
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.
-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.
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