On Fri, Aug 5, 2016 at 7:33 PM, William A Rowe Jr <[email protected]> wrote: > On Fri, Aug 5, 2016 at 10:43 AM, Yann Ylavic <[email protected]> wrote: >> >> @@ -903,8 +903,16 @@ AP_DECLARE(void) ap_get_mime_headers_core(request_ >> */ >> continue; >> } >> - else if (last_field != NULL) { >> >> + if (last_field == NULL) { >> + /* Keep track of this first header line so that we can extend >> it >> + * across any obs-fold or parse it on the next loop >> iteration. >> + */ >> + last_field = field; >> + last_len = len; >> + continue; >> + } >> + >> /* Process the previous last_field header line with all >> obs-folded >> * segments already concatinated (this is not operating on >> the >> * most recently read input line). > > > This patch makes it less clear that the continue; case above also avoided > the empty-line case (that was clearer in the main loop imo),
Personnaly an if-continued followed by an else make me think that there is (at least) one top if-not-continued in the series (often hard to follow construction), but this isn't the case here... Anyway, I don't like too much my patch finally because it duplicates logic code (few but still). > while adding > the unnecessary verification of last_len != 0 from line 904, so I'd say it's > a > net loss of legibility in spite of gaining us 4 characters. Just my 2c. The 'len' verification is necessary because it can be zero when last_field is NULL (legitimately, no headers at all), and we must leave still.
