#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:24 kevin8t8]:
> 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.
I was arguing that Vincent's point about underspecifying the array
essentially applied to both the fixed-length array and the macro cases;
therefore on that basis specifically, neither solution was better than the
other--other factors needed to be considered.
> 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 just don't agree. I think the nested macros are much too hard to parse,
and apparently depends on a non-standard compiler extension? Clever code
is not good code. Code that is easy to read and understand is good code.
> 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.
You could remove that as well. You could instead have your helper
function realloc the menu buffer, or you could also use the snprintf()
trick in the snprintf man page to copy the whole pile of strings at once,
and do at most one realloc(). The check_cert() function could be
similarly adjusted... realloc() a lot of data isn't awesome but we're
talking about a small number of reallocs with relatively small strings...
it shouldn't really matter too much. Or you could use an array of char*
where each element is a dynamically allocated row, where the last one is
NULL. Then the only thing you might need to realloc is additional pointer
elements.
There are many ways to solve that problem.
> I think mutt_array_size() accomplishes this marginally better than the
helper function.
I don't agree, but I've said my piece, and it's time to move on.
For what it's worth, I thought the original patch was adequate. The only
reason I'm pursuing this is because Vincent objected for reasons of code
quality, and I don't think the macro is in any way an improvement. I
think writing good code is the right goal; but if you're going to take
that seriously, then take it seriously. :)
--
Ticket URL: <https://dev.mutt.org/trac/ticket/3899#comment:28>
Mutt <http://www.mutt.org/>
The Mutt mail user agent
