On Wed, Jun 3, 2015 at 8:01 AM, Ángel González <keisial@xxxxxxxxx> wrote:
> On 02/06/15 15:46, György Demarcsek Ifj. wrote:
>
[...]
> You also removed a number of trailing spaces from the files, which make
> the patch harder to read.
>
We should clean up the trailing spaces upstream (there's more than I would
have thought) but that churn can wait until after the upcoming release. I
agree that having no-op whitespace changes in the diff doesn't help.
Anyway, as to the diff itself:
+ if (authctxt->last_auth_methods == NULL) {
+ authctxt->last_auth_methods = xcalloc(strlen(method) + 2,
sizeof(char));
sizeof(char) == 1 by definition.
+ } else {
+ am_copy = xstrdup(authctxt->last_auth_methods);
+ free(authctxt->last_auth_methods);
+ authctxt->last_auth_methods = xcalloc(strlen(am_copy) + strlen(method)
+ 2, sizeof(char));
+ strcpy(authctxt->last_auth_methods, am_copy);
strcpy (and strcat) are not bounds checked (probably safe in this
particular case but we consider their use to be bad practice).
+ free(am_copy);
+ }
+
+ strcat(authctxt->last_auth_methods, method);
+ if (authctxt->num_auth_methods != 1)
+ strcat(authctxt->last_auth_methods, ",");
This seems to be an extremely complicated way of saying "append the method
to a comma separated list" (although it'll have a trailing comma, which I'm
not sure is intended. How about something like:
if (authctxt->last_auth_methods == NULL)
authctxt->last_auth_methods = xstrdup(method);
else {
am_copy = authctxt->last_auth_methods;
xasprintf(&authctxt->last_auth_methods, "%s,%s", am_copy, method);
free(am_copy);
}
--
Darren Tucker (dtucker at zip.com.au)
GPG key 8FF4FA69 / D9A3 86E9 7EEE AF4B B2D4 37C9 C982 80C7 8FF4 FA69
Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@xxxxxxxxxxx
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev
