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

[Oleg Gurevich]

Hi Justus,

thank you for comments. I appreciate it. It is decent and makes sense.
... will pay attention next time and hope it will get better.


mit freundlichen Grüßen/ с уважением/ best regards

Oleg Gurevich

PGP Key: E74A0B0C
PGP fingerprint: 38A0 D0CC BD23 1707 B0AF  D158 E9D7 6E3F E74A 0B0C

On 12/08/2015 02:03 PM, Justus Winter wrote:
> 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
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: </pipermail/attachments/20151208/068deceb/attachment.sig>