[squid-dev] [PATCH] Coverity-fixes, part 1: EnumIterator

[Kinkie]

On Mon, Aug 24, 2015 at 8:08 PM, Amos Jeffries <squid3 at treenet.co.nz> wrote:

> On 25/08/2015 4:49 a.m., Kinkie wrote:
> > Hi,
> >    this is the first of a series of patches stemming from the
> > coverity-fixes effort.
> > The aim of this first submission  is to implement WholeEnum and EnumRange
> > and supporting unit tests.
> >
> > Big-context patch attached
> >
>
> in configure.ac:
> * no need for two lines, just add the macro straight after the list of
> other types.
>
> Ok.


>
> in src/Makefile.am:
> * since this is scoped at the enum operator please remove the
> nodist_tests_testX_SOURCES change
>  - I will pick that up in the Makefile.am polishing later
>

Ok


>
> * please do omit tests_testEnumIterator_DEPENDENCIES
>  - all it does now is harm repeatability of your test results
>
> Ok


>
> in src/base/EnumIterator.h:
> * the description of requirements for the marker on WholeEnum is a
> little bit mind-boggling.
>  - a bullet list might be better here if you can find the doxygen syntax
> for that. IIRC it was ' - ', but double check if it needs empty lines
> between.
>

Done. Yes it's a minus, no it doesn't need empty lines.


>
> * the iterator_type being used in static_cast before the typedef _looks_
> very dangerous (even if its fine and works).
>  - I would place typedef up the top in their own protected area to avoid
> a) depending on implicit compiler behaviour and b) future developer
> mistakes/misunderstanding.
>

Ok. Done.


> I dont see the first-user code that we normally require for accepting
> these. I know its coming with a rather big followup, just would be good
> if there was a simple use that could be used as exemplar.
>

as icp_opcode. Instrumentation needed for iterators has been implemented
alongside the existing ICP_MAX value which is apparently (and
unfortunately) loaded with more than one meaning.

I caught a limitation in mk-string-arrays which looks for "typedef" instead
of "enum" to detect the start of the symbol definitions; fixed that adding
support for "enum enumname" and "enum class enumname" enumBegin_ and
enumEnd_ are not caught because the script requires the first letter of the
enum symbol to be capitalized.

v2 patch attached.

Thanks!

-- 
    Francesco
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squid-cache.org/pipermail/squid-dev/attachments/20150826/af3ed3b4/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: coverity-fixes-enumiterator-v2.patch
Type: application/octet-stream
Size: 29363 bytes
Desc: not available
URL: <http://lists.squid-cache.org/pipermail/squid-dev/attachments/20150826/af3ed3b4/attachment-0001.obj>