On Sat, 3 Nov 2012 08:50:29 -0400
Jeff Layton <jlayton@xxxxxxxxx> wrote:
> One of the reasons to use "goto" in an error condition is to eliminate
> unnecessary indentation. Fix that here by revering some error checks
> end getting rid of some unneeded "else" cases.
>
> After using strstr() to find "ACL:", there's no need to then use
> strchr() to find ':'. We know where it is -- it's 3 bytes past the
> current position.
>
> Finally, there's no need to copy these strings into new buffers,
> just set the pointers in the array to their original values.
>
> Signed-off-by: Jeff Layton <jlayton@xxxxxxxxx>
> ---
> setcifsacl.c | 50 ++++++++++++++++++--------------------------------
> 1 file changed, 18 insertions(+), 32 deletions(-)
>
> diff --git a/setcifsacl.c b/setcifsacl.c
> index faf5405..54d8cbc 100644
> --- a/setcifsacl.c
> +++ b/setcifsacl.c
> @@ -655,48 +655,36 @@ build_cmdline_aces_ret:
> }
>
> static char **
> -parse_cmdline_aces(char *optarg, int numcaces)
> +parse_cmdline_aces(char *acelist, int numcaces)
> {
> int i = 0, len;
> char *acestr, *vacestr, **arrptr = NULL;
>
> - errno = EINVAL;
> arrptr = (char **)malloc(numcaces * sizeof(char *));
> if (!arrptr) {
> - printf("%s: Error %d allocating char array\n", __func__, errno);
> + printf("%s: Unable to allocate char array\n", __func__);
> return NULL;
> }
>
> while (i < numcaces) {
> - acestr = strtok(optarg, ","); /* everything before , */
> - if (acestr) {
> - vacestr = strstr(acestr, "ACL:"); /* ace as ACL:*" */
> - if (vacestr) {
> - vacestr = strchr(vacestr, ':');
> - if (vacestr)
> - ++vacestr; /* go past : */
> - if (vacestr) {
> - len = strlen(vacestr);
> - arrptr[i] = malloc(len + 1);
> - if (!arrptr[i])
> - goto parse_cmdline_aces_ret;
> - strcpy(arrptr[i], vacestr);
> - ++i;
> - } else
> - goto parse_cmdline_aces_ret;
> - } else
> - goto parse_cmdline_aces_ret;
> - } else
> - goto parse_cmdline_aces_ret;
> - optarg = NULL;
> - }
> - errno = 0;
> + acestr = strtok(acelist, ","); /* everything before , */
> + if (!acestr)
> + goto parse_cmdline_aces_err;
> +
> + vacestr = strstr(acestr, "ACL:"); /* ace as ACL:*" */
> + if (!vacestr)
> + goto parse_cmdline_aces_err;
> + vacestr += 4; /* skip past "ACL:" */
> + if (vacestr) {
Oops: that should be "if (*vacestr)". Will fix...
> + arrptr[i] = vacestr;
> + ++i;
> + }
> + acelist = NULL;
> + }
> return arrptr;
>
> -parse_cmdline_aces_ret:
> - printf("%s: Error %d parsing ACEs\n", __func__, errno);
> - for (; i >= 0; --i)
> - free(arrptr[i]);
> +parse_cmdline_aces_err:
> + printf("%s: Error parsing ACEs\n", __func__);
> free(arrptr);
> return NULL;
> }
> @@ -908,8 +896,6 @@ setcifsacl_cmdlineverify_ret:
> free(cacesptr);
>
> setcifsacl_cmdlineparse_ret:
> - for (i = 0; i < numcaces; ++i)
> - free(arrptr[i]);
> free(arrptr);
>
> setcifsacl_numcaces_ret:
--
Jeff Layton <jlayton@xxxxxxxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
