#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 kevin8t8):

 I figured out my confusion.  !__must_be_array() in the Linux source was
 comparing "a with &a[0]", not "&a with &a[0]".

 So the meaning of the macro is BUILD_BUG_ON "if the parameter is true",
 "otherwise return" ZERO.

 Derek, I understand your general point, but I don't follow your logic in
 comment:18.  Saying you can just as easily make the array too small just
 sounds argumentative to me.  Your comment:19 shows that it's just as easy
 to goof up the helper function.  Both of these goof ups will show up with
 a quick test, but I thought we were more discussing general code clarity
 and maintainability here.

 Again, I completely agree with your comment:10, but if we can avoid
 misuse, there are situations where a mutt_array_size() makes the code
 clearer, more concise, and less buggy.  I wouldn't want to see it all over
 the code, but for small self-contained functions I think it makes sense.
 Yes, your helper function is perhaps clearer than the loop, but on the
 other hand, the setting of menu->max is more brittle, and easier to forget
 to change if a field is added to the helper.

 Now, that said, I think hardcoding the menu->max beforehand is crap code,
 but I'm trying to minimize my changes to this function, make it a little
 better maintainable for the future, and move on.  I think
 mutt_array_size() accomplishes this marginally better than the helper
 function.

 Changing topics to the macro, the "BUILD_BUG" macro is pretty clever.  It
 takes the size of a struct with a bit field.  A zero-length bit field will
 end up returning a 0 size, but a negative bit field length is illegal and
 will generate a compiler error.

 The "must_be_array" uses Vincents comparison to make sure "a" is an array
 and a not an array pointer.

 The mutt_array_size() is modified to add the result of "must_be_array",
 which will either return 0 or will generate a compilation error.

 It's a little complicated, but not too bad (with a few comments thrown
 in).  I'm curious what Vincent's feedback is though, and if he has any
 better ideas.

--
Ticket URL: <https://dev.mutt.org/trac/ticket/3899#comment:24>
Mutt <http://www.mutt.org/>
The Mutt mail user agent

Reply via email to