Re: [fetchmail-devel] fetchmail fix coordination and future planning

[Sunil S. <sun...@ro...>]

----- Original Message -----

> Observations:
> 
> - the patch makes an undocumented assumption that the buffer for a
> second call to gen_recv_split is always large enough to copy the
> remainder. If it isn't, you lose data.  Doesn't happen in fetchmail now,
> but is a maintenance concern, so I'll add an assert() for now.
> 
> - you don't guard the prefix length - I've added that.
> 
> - you don't check if the data-to-be-cached fits into the rs structure -
> I've added that.

Good. All required.

> - /** foo */ isn't a typo, but a marker for Doxygen so it actually looks
> at the comment for documentation, and extracts it. Also note the
> ordering of these comments versus the comma; alternatively you can write
> /**< foo */ to document the PREVIOUS argument in a prototype.

Okay. Note that the prototype was just copied from gen_recv() and a parameter was added later.

>     Fixup: match prefix caseblind, add some guards, streamline phase
> handling.

Required.

>     Add a few asserts to catch abuse, and use strlcpy/strlcat rather than
>     snprintf because some implementations of the latter are unsuitable for
>     detecting buffer exhaustion.

Required.

>     Fixup: remove unused variables.

How did I miss that?

Thanks for the review.

-- 
Sunil Shetye.