On Fri, Jul 27, 2012 at 06:44:04PM +0000, Blue Swirl wrote: > > +struct GlusterOpts { > > static
Sure. > > > + bool optional; > > + char defval[10]; > > const char *defval? Sure I could. > > > + char *value; > > +} GlusterOpts[] = { > > + {false, "", NULL }, > > + {false, "", NULL }, > > + {true, "0", NULL }, > > + {true, "socket", NULL }, > > + {false, "", NULL }, > > + {false, "", NULL }, > > +}; > > + > > + if (i == GOPT_LAST-1 && strlen(q)) { > > Spaces around '-'. checkpatch.pl doesn't enforce this, but I can change. > > + > > + port = strtoul(GlusterOpts[GOPT_PORT].value, NULL, 0); > > + if (port < 0) { > > port > 65535 could be bad too. Actually I am just checking if strtoul gave me a valid integer only and depending on gluster to flag an error for invalid port number. But I guess no harm in checking for valid port range here. Is there a #define equivalent for 65535 ? > > > + > > + fd = glfs_creat(glfs, GlusterOpts[GOPT_IMAGE].value, > > + O_WRONLY|O_CREAT|O_TRUNC|O_BINARY, S_IRUSR|S_IWUSR); > > Spaces around '|'. Again, checkpatch.pl doesn't enforce this, but I can change. Thanks for take time to review. Regards, Bharata.