[PATCH v5 1/4] libusbg: Replace array lengths with defines

2014-01-22 Thread Stanislaw Wadas
Replace hard coded value of 256 by two constant
defines, MAX_LENGTH and MAX_PATH_LENGTH

Signed-off-by: Stanislaw Wadas s.wa...@samsung.com
---
 include/usbg/usbg.h |   27 +++
 src/usbg.c  |   46 +++---
 2 files changed, 38 insertions(+), 35 deletions(-)

diff --git a/include/usbg/usbg.h b/include/usbg/usbg.h
index 5f00068..abb9bc2 100644
--- a/include/usbg/usbg.h
+++ b/include/usbg/usbg.h
@@ -33,13 +33,16 @@
 #define DEFAULT_UDCNULL
 #define LANG_US_ENG0x0409
 
+#define MAX_LENGTH 256
+#define MAX_PATH_LENGTH 256
+
 /**
  * @struct state
  * @brief State of the gadget devices in the system
  */
 struct state
 {
-   char path[256];
+   char path[MAX_PATH_LENGTH];
 
TAILQ_HEAD(ghead, gadget) gadgets;
 };
@@ -51,8 +54,8 @@ struct state
 struct gadget
 {
char name[40];
-   char path[256];
-   char udc[256];
+   char path[MAX_PATH_LENGTH];
+   char udc[MAX_LENGTH];
int dclass;
int dsubclass;
int dproto;
@@ -61,9 +64,9 @@ struct gadget
int bcdusb;
int product;
int vendor;
-   char str_ser[256];
-   char str_mnf[256];
-   char str_prd[256];
+   char str_ser[MAX_LENGTH];
+   char str_mnf[MAX_LENGTH];
+   char str_prd[MAX_LENGTH];
TAILQ_ENTRY(gadget) gnode;
TAILQ_HEAD(chead, config) configs;
TAILQ_HEAD(fhead, function) functions;
@@ -81,10 +84,10 @@ struct config
struct gadget *parent;
 
char name[40];
-   char path[256];
+   char path[MAX_PATH_LENGTH];
int maxpower;
int bmattrs;
-   char str_cfg[256];
+   char str_cfg[MAX_LENGTH];
 };
 
 /**
@@ -136,7 +139,7 @@ struct serial_attrs {
 struct net_attrs {
struct ether_addr dev_addr;
struct ether_addr host_addr;
-   char ifname[256];
+   char ifname[MAX_LENGTH];
int qmult;
 };
 
@@ -145,7 +148,7 @@ struct net_attrs {
  * @brief Attributes for the phonet USB function
  */
 struct phonet_attrs {
-   char ifname[256];
+   char ifname[MAX_LENGTH];
 };
 
 /**
@@ -168,7 +171,7 @@ struct function
struct gadget *parent;
 
char name[40];
-   char path[256];
+   char path[MAX_PATH_LENGTH];
 
enum function_type type;
union attrs attr;
@@ -187,7 +190,7 @@ struct binding
struct function *target;
 
char name[40];
-   char path[256];
+   char path[MAX_PATH_LENGTH];
 };
 
 /* Library init and cleanup */
diff --git a/src/usbg.c b/src/usbg.c
index 20447db..cb7f172 100644
--- a/src/usbg.c
+++ b/src/usbg.c
@@ -82,7 +82,7 @@ static int file_select(const struct dirent *dent)
 
 static char *usbg_read_buf(char *path, char *name, char *file, char *buf)
 {
-   char p[256];
+   char p[MAX_LENGTH];
FILE *fp;
char *ret = NULL;
 
@@ -92,7 +92,7 @@ static char *usbg_read_buf(char *path, char *name, char 
*file, char *buf)
if (!fp)
goto out;
 
-   ret = fgets(buf, 256, fp);
+   ret = fgets(buf, MAX_LENGTH, fp);
 
fclose(fp);
 
@@ -102,7 +102,7 @@ out:
 
 static int usbg_read_int(char *path, char *name, char *file, int base)
 {
-   char buf[256];
+   char buf[MAX_LENGTH];
 
if (usbg_read_buf(path, name, file, buf))
return strtol(buf, NULL, base);
@@ -125,7 +125,7 @@ static void usbg_read_string(char *path, char *name, char 
*file, char *buf)
 
 static void usbg_write_buf(char *path, char *name, char *file, char *buf)
 {
-   char p[256];
+   char p[MAX_LENGTH];
FILE *fp;
 
sprintf(p, %s/%s/%s, path, name, file);
@@ -143,7 +143,7 @@ static void usbg_write_buf(char *path, char *name, char 
*file, char *buf)
 
 static void usbg_write_int(char *path, char *name, char *file, int value, char 
*str)
 {
-   char buf[256];
+   char buf[MAX_LENGTH];
 
sprintf(buf, str, value);
usbg_write_buf(path, name, file, buf);
@@ -196,7 +196,7 @@ static int usbg_parse_functions(char *path, struct gadget 
*g)
struct function *f;
int i, n;
struct dirent **dent;
-   char fpath[256];
+   char fpath[MAX_PATH_LENGTH];
 
sprintf(fpath, %s/%s/functions, path, g-name);
 
@@ -227,7 +227,7 @@ static void usbg_parse_config_bindings(struct config *c)
 {
int i, n;
struct dirent **dent;
-   char bpath[256];
+   char bpath[MAX_PATH_LENGTH];
struct gadget *g = c-parent;
struct binding *b;
struct function *f;
@@ -240,12 +240,12 @@ static void usbg_parse_config_bindings(struct config *c)
for (i=0; i  n; i++) {
TAILQ_FOREACH(f, g-functions, fnode) {
int n;
-   char contents[256];
-   char cpath[256];
+   char contents[MAX_LENGTH];
+   char cpath[MAX_PATH_LENGTH];
char fname[40];
 
  

Re: [PATCH v5 1/4] libusbg: Replace array lengths with defines

2014-01-22 Thread Matt Porter
On Wed, Jan 22, 2014 at 02:56:20PM +0100, Stanislaw Wadas wrote:
 Replace hard coded value of 256 by two constant
 defines, MAX_LENGTH and MAX_PATH_LENGTH
 
 Signed-off-by: Stanislaw Wadas s.wa...@samsung.com
 ---
  include/usbg/usbg.h |   27 +++
  src/usbg.c  |   46 +++---
  2 files changed, 38 insertions(+), 35 deletions(-)
 
 diff --git a/include/usbg/usbg.h b/include/usbg/usbg.h
 index 5f00068..abb9bc2 100644
 --- a/include/usbg/usbg.h
 +++ b/include/usbg/usbg.h
 @@ -33,13 +33,16 @@
  #define DEFAULT_UDC  NULL
  #define LANG_US_ENG  0x0409
  
 +#define MAX_LENGTH 256

This should be MAX_STR_LENGTH to clarify the use as mentioned
before.

 +#define MAX_PATH_LENGTH 256
 +
  /**
   * @struct state
   * @brief State of the gadget devices in the system
   */
  struct state
  {
 - char path[256];
 + char path[MAX_PATH_LENGTH];
  
   TAILQ_HEAD(ghead, gadget) gadgets;
  };
 @@ -51,8 +54,8 @@ struct state
  struct gadget
  {
   char name[40];
 - char path[256];
 - char udc[256];
 + char path[MAX_PATH_LENGTH];
 + char udc[MAX_LENGTH];

and MAX_STR_LENGTH here and throughout

   int dclass;
   int dsubclass;
   int dproto;
 @@ -61,9 +64,9 @@ struct gadget
   int bcdusb;
   int product;
   int vendor;
 - char str_ser[256];
 - char str_mnf[256];
 - char str_prd[256];
 + char str_ser[MAX_LENGTH];
 + char str_mnf[MAX_LENGTH];
 + char str_prd[MAX_LENGTH];
   TAILQ_ENTRY(gadget) gnode;
   TAILQ_HEAD(chead, config) configs;
   TAILQ_HEAD(fhead, function) functions;
 @@ -81,10 +84,10 @@ struct config
   struct gadget *parent;
  
   char name[40];
 - char path[256];
 + char path[MAX_PATH_LENGTH];
   int maxpower;
   int bmattrs;
 - char str_cfg[256];
 + char str_cfg[MAX_LENGTH];
  };
  
  /**
 @@ -136,7 +139,7 @@ struct serial_attrs {
  struct net_attrs {
   struct ether_addr dev_addr;
   struct ether_addr host_addr;
 - char ifname[256];
 + char ifname[MAX_LENGTH];
   int qmult;
  };
  
 @@ -145,7 +148,7 @@ struct net_attrs {
   * @brief Attributes for the phonet USB function
   */
  struct phonet_attrs {
 - char ifname[256];
 + char ifname[MAX_LENGTH];
  };
  
  /**
 @@ -168,7 +171,7 @@ struct function
   struct gadget *parent;
  
   char name[40];
 - char path[256];
 + char path[MAX_PATH_LENGTH];
  
   enum function_type type;
   union attrs attr;
 @@ -187,7 +190,7 @@ struct binding
   struct function *target;
  
   char name[40];
 - char path[256];
 + char path[MAX_PATH_LENGTH];
  };
  
  /* Library init and cleanup */
 diff --git a/src/usbg.c b/src/usbg.c
 index 20447db..cb7f172 100644
 --- a/src/usbg.c
 +++ b/src/usbg.c
 @@ -82,7 +82,7 @@ static int file_select(const struct dirent *dent)
  
  static char *usbg_read_buf(char *path, char *name, char *file, char *buf)
  {
 - char p[256];
 + char p[MAX_LENGTH];
   FILE *fp;
   char *ret = NULL;
  
 @@ -92,7 +92,7 @@ static char *usbg_read_buf(char *path, char *name, char 
 *file, char *buf)
   if (!fp)
   goto out;
  
 - ret = fgets(buf, 256, fp);
 + ret = fgets(buf, MAX_LENGTH, fp);
  
   fclose(fp);
  
 @@ -102,7 +102,7 @@ out:
  
  static int usbg_read_int(char *path, char *name, char *file, int base)
  {
 - char buf[256];
 + char buf[MAX_LENGTH];
  
   if (usbg_read_buf(path, name, file, buf))
   return strtol(buf, NULL, base);
 @@ -125,7 +125,7 @@ static void usbg_read_string(char *path, char *name, char 
 *file, char *buf)
  
  static void usbg_write_buf(char *path, char *name, char *file, char *buf)
  {
 - char p[256];
 + char p[MAX_LENGTH];
   FILE *fp;
  
   sprintf(p, %s/%s/%s, path, name, file);
 @@ -143,7 +143,7 @@ static void usbg_write_buf(char *path, char *name, char 
 *file, char *buf)
  
  static void usbg_write_int(char *path, char *name, char *file, int value, 
 char *str)
  {
 - char buf[256];
 + char buf[MAX_LENGTH];
  
   sprintf(buf, str, value);
   usbg_write_buf(path, name, file, buf);
 @@ -196,7 +196,7 @@ static int usbg_parse_functions(char *path, struct gadget 
 *g)
   struct function *f;
   int i, n;
   struct dirent **dent;
 - char fpath[256];
 + char fpath[MAX_PATH_LENGTH];
  
   sprintf(fpath, %s/%s/functions, path, g-name);
  
 @@ -227,7 +227,7 @@ static void usbg_parse_config_bindings(struct config *c)
  {
   int i, n;
   struct dirent **dent;
 - char bpath[256];
 + char bpath[MAX_PATH_LENGTH];
   struct gadget *g = c-parent;
   struct binding *b;
   struct function *f;
 @@ -240,12 +240,12 @@ static void usbg_parse_config_bindings(struct config *c)
   for (i=0; i  n; i++) {
   TAILQ_FOREACH(f, g-functions, fnode) {
   int n;
 - char contents[256];
 -

RE: [PATCH v5 1/4] libusbg: Replace array lengths with defines

2014-01-22 Thread David Laight
From: Stanislaw Wadas
 Replace hard coded value of 256 by two constant
 defines, MAX_LENGTH and MAX_PATH_LENGTH

Neither of those names is really very good.
They probably ought to be prefixed with USBG_
Mind you the rest of the file isn't much better.

There are also some 'char name[40];'

And code like:

  static void usbg_write_buf(char *path, char *name, char *file, char *buf)
  {
 - char p[256];
 + char p[MAX_LENGTH];
   FILE *fp;
 
   sprintf(p, %s/%s/%s, path, name, file);

Is just waiting for a security alert.

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 v5 1/4] libusbg: Replace array lengths with defines

2014-01-22 Thread Krzysztof Opasiak
 -Original Message-
 From: linux-usb-ow...@vger.kernel.org [mailto:linux-usb-
 ow...@vger.kernel.org] On Behalf Of David Laight
 Sent: Wednesday, January 22, 2014 3:16 PM
 To: Stanislaw Wadas; matt.por...@linaro.org
 Cc: linux-usb@vger.kernel.org; Piotr Bereza;
 myungjoo@samsung.com; Marek Szyprowski;
 kyungmin.p...@samsung.com; Krzysztof Opasiak; Andrzej Pietrasiewicz
 Subject: RE: [PATCH v5 1/4] libusbg: Replace array lengths with
 defines
 
 From: Stanislaw Wadas
  Replace hard coded value of 256 by two constant
  defines, MAX_LENGTH and MAX_PATH_LENGTH
 
 Neither of those names is really very good.
 They probably ought to be prefixed with USBG_
 Mind you the rest of the file isn't much better.

Yes, I would also suggest to make this USBG_MAX_STR_LENGTH and
USBG_MAX_PATH_LENGTH.

 
 There are also some 'char name[40];'

Maybe some USBG_MAX_NAME_LENGTH would be suitable here? What do you
think Matt?

 
 And code like:
 
   static void usbg_write_buf(char *path, char *name, char *file,
 char *buf)
   {
  -   char p[256];
  +   char p[MAX_LENGTH];
  FILE *fp;
 
  sprintf(p, %s/%s/%s, path, name, file);
 
 Is just waiting for a security alert.

Yes that's true. In future versions this will be fixed with more secure
mechanism.

--
BR's 
Krzysztof Opasiak
Samsung RD Institute Poland
Samsung Electronics
k.opas...@samsung.com




--
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