Re: [Mutt] #3899: mutt_ssl's interactive_check_cert() has several issues

[Mutt]

#3899: mutt_ssl's interactive_check_cert() has several issues
---+--
  Reporter:  kevin8t8  |  Owner:  mutt-dev
  Type:  defect| Status:  closed
  Priority:  major |  Milestone:
 Component:  crypto|Version:
Resolution:  fixed |   Keywords:
---+--

Comment (by derekmartin):

 Oops, the dialog[] index needs to be (*row)++...

--
Ticket URL: 
Mutt 
The Mutt mail user agent

Re: [Mutt] #3899: mutt_ssl's interactive_check_cert() has several issues

[Mutt]

#3899: mutt_ssl's interactive_check_cert() has several issues
---+--
  Reporter:  kevin8t8  |  Owner:  mutt-dev
  Type:  defect| Status:  closed
  Priority:  major |  Milestone:
 Component:  crypto|Version:
Resolution:  fixed |   Keywords:
---+--

Comment (by derekmartin):

 Replying to [comment:17 vinc17]:
 > That would be OK, but the code needs to check that the initializer has
 enough elements, as the C standard allows you to give fewer elements than
 the real size (the other ones are initialized to 0).

 The macro version has the opposite problem: You can make the array too
 small and it will "work" but it will be incomplete.  Ultimately, a bug is
 a bug, and you just have to write the code correctly.  The compiler and
 coding conventions can only do so much for you.

 Besides, like I said, the better solution is to forgo both the array and
 the struct replacement, and just write a helper function that does the
 sprintf.  It is more explicit and can't be wrong.

 Add this helper function:

 {{{
 static void sprintf_cert_menu(MUTTMENU *menu, X509_NAME *n, int *row)
 {
 snprintf(menu->dialog[*row++], SHORT_STRING, "  %s", x509_get_part(n,
 NID_commonName);
 snprintf(menu->dialog[*row++], SHORT_STRING, "  %s", x509_get_part(n,
 NID_pkcs9_emailAddress);
 snprintf(menu->dialog[*row++], SHORT_STRING, "  %s", x509_get_part(n,
 NID_organizationName);
 snprintf(menu->dialog[*row++], SHORT_STRING, "  %s", x509_get_part(n,
 NID_organizationalUnitName);
 snprintf(menu->dialog[*row++], SHORT_STRING, "  %s", x509_get_part(n,
 NID_localityName);
 snprintf(menu->dialog[*row++], SHORT_STRING, "  %s", x509_get_part(n,
 NID_stateOrProvince);
 snprintf(menu->dialog[*row++], SHORT_STRING, "  %s", x509_get_part(n,
 NID_countryName);
 }

 }}}

 Then remove the int array part[], and replace both occurances of the loop
 structure to snprintf() the parts with a call to the helper:

 {{{
 sprintf_cert_menu(menu, x509_subject, );
 [...]
 sprintf_cert_menu(menu, x509_issuer, );
 }}}

 Done and done.  Less code, more explicit, no need for any macro
 contortions, can't be wrong (and still compile).  I'm only unsure of
 whether or not the x509 functions can fail, as I'm unable to find man
 pages for x509_get_*_name() functions...

--
Ticket URL: 
Mutt 
The Mutt mail user agent

Re: [Mutt] #3899: mutt_ssl's interactive_check_cert() has several issues

[Mutt]

#3899: mutt_ssl's interactive_check_cert() has several issues
---+--
  Reporter:  kevin8t8  |  Owner:  mutt-dev
  Type:  defect| Status:  closed
  Priority:  major |  Milestone:
 Component:  crypto|Version:
Resolution:  fixed |   Keywords:
---+--

Comment (by vinc17):

 Replying to [comment:10 derekmartin]:
 > Better would be something like:
 >
 > {{{
 > #define CERT_CHECK_PARTS 7
 > typedef int cert_check_parts[CERT_CHECK_PARTS];
 > [...]
 > const cert_check_parts part = { ... };
 > [...]
 > for (i = 0; i < CERT_CHECK_PARTS; ++i) ...
 > }}}
 That would be OK, but the code needs to check that the initializer has
 enough elements, as the C standard allows you to give fewer elements than
 the real size (the other ones are initialized to 0).

--
Ticket URL: 
Mutt 
The Mutt mail user agent

Re: [Mutt] #3899: mutt_ssl's interactive_check_cert() has several issues

[Mutt]

#3899: mutt_ssl's interactive_check_cert() has several issues
---+--
  Reporter:  kevin8t8  |  Owner:  mutt-dev
  Type:  defect| Status:  closed
  Priority:  major |  Milestone:
 Component:  crypto|Version:
Resolution:  fixed |   Keywords:
---+--

Comment (by vinc17):

 Replying to [comment:10 derekmartin]:
 > Replying to [comment:2 vinc17]:
 > > {{{
 > > #define numberof(x)  (sizeof (x) / sizeof ((x)[0]))
 > >
 > > for (i = 0; i < numberof(parts); i++)
 > > }}}
 >
 > I think this kind of macro, much like the macro in bug #3880, is bad
 business.  What if it gets used with a pointer rather than an array?  The
 syntax is still fine, it'll compile, but it will break, in a way that may
 be difficult to notice, depending on exactly how it's used.

 The macro can be modified to check whether the variable is a pointer. So,
 I don't see any problem.

--
Ticket URL: 
Mutt 
The Mutt mail user agent