On Scute v1.4.0 support key length up to 4096, proceed s-expressions on common way

Justus Winter justus at g10code.com
Tue Dec 8 14:03:59 CET 2015


Hi Oleg,

Quoting Oleg Gurevich (2015-11-18 19:02:28)
> would you please take a look on following patch.

thanks for your patch.  I cannot comment on the big picture, but I can
nitpick a little.

> +struct s_expression {

We follow the GNU coding conventions, see doc/HACKING for details.
Please put the opening bracket on a new line here.

> +    unsigned int sig_len;
> +    char *prefix;
> +    unsigned int prefix_len;
> +

Spurious newline.

> +} supported_expr[] = {

Likewise insert a newline here please.

> +  /* FIXME: better ? */
> +  err = GPG_ERR_BAD_SIGNATURE;
> +  for(i = 0; i < sizeof(supported_expr) / sizeof(struct s_expression); i++)

sizeof is an operator and hence requires no parenthesis in general.
If required, please add a space before opening parenthesis.  Maybe we
even have a version of ARRAY_SIZE, I don't know tbh.

>      {
> -      if (sig.len != SIG_PREFIX_LEN + SIG_LEN + SIG_POSTFIX_LEN)
> -        return gpg_error (GPG_ERR_BAD_SIGNATURE);
> -      if (memcmp (sig.data, SIG_PREFIX, SIG_PREFIX_LEN))
> -        return gpg_error (GPG_ERR_BAD_SIGNATURE);
> -      if (memcmp (sig.data + sig.len - SIG_POSTFIX_LEN,
> -                  SIG_POSTFIX, SIG_POSTFIX_LEN))
> -        return gpg_error (GPG_ERR_BAD_SIGNATURE);
> -      memcpy (sig_result, sig.data + SIG_PREFIX_LEN, SIG_LEN);
> -      *sig_len = SIG_LEN;
> +      if( sig.len == supported_expr[i].prefix_len +

Personally, I like 'if (expr)' better than 'if( expr )', and even
though both is used in gnupg, I the former is used more often.  In
general, try to make your changes blend in with the code surrounding
it.  Likewise for 'for'.

> supported_expr[i].sig_len + SIG_POSTFIX_LEN )

Note that your patch got mangled by your MUA.  Consider using git
send-email.

> +        {
> +          // check prefix matching

No C++ comments please.

> +          if (memcmp (sig.data, supported_expr[i].prefix,
> supported_expr[i].prefix_len))

Likewise mangled here.

> +            return gpg_error (GPG_ERR_BAD_SIGNATURE);
> +          // check postfix matching
> +          if(memcmp(sig.data + sig.len - SIG_POSTFIX_LEN, SIG_POSTFIX,

Space before the '('s.


Cheers,
Justus



More information about the Gnupg-devel mailing list