maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Jan!

On Mar 18, Jan Lindström wrote:
> revision-id: 2bdbfd334bba93f75b2952e8e76f20ed29fb2bd4
> parent(s): c14d9c21f0a6974d9909e00a23522114caad3bfc
> committer: Jan Lindström
> branch nick: 10.0-git
> timestamp: 2015-03-18 12:18:39 +0200
> message:
> 
> Fix assertion failure seen on Buildbot win32-debug
> 
> There is a bug in Visual Studio 2010
> Visual Studio has a feature "Checked Iterators". In a debug build, every
> iterator operation is checked at runtime for errors, e g, out of range.
> Disable this "Checked Iterators" for Windows and Debug if defined.

This comment,

> +/* There is a bug in Visual Studio 2010
> +Visual Studio has a feature "Checked Iterators". In a debug build, every
> +iterator operation is checked at runtime for errors, e g, out of range.
> +Disable this "Checked Iterators" for Windows and Debug if defined.
> +*/

and this comment don't explain what bug is it. They create an impression
that Visual Studio has run-time checks for an iterator running out of
range. And that our code fails this check (because the iterator runs out
of range). And that your "fix" is to disable the check instead of fixing
the buggy out-of-range iterator.

I'm not saying that's what happens, I'm saying that your comment
suggests that this happens, because you didn't describe what was the
actual bug. A better comment should say that our iterator is correct,
but a buggy check erroneously marks it as out-of-range. It should
explain what iterator is it, exactly, why it's correct, etc.

Could you please fix the comment please? (or the iterator, whatever
should be fixed)

> +#ifdef UNIV_DEBUG
> +#ifdef __WIN__
> +#ifdef _ITERATOR_DEBUG_LEVEL
> +#define _ITERATOR_DEBUG_LEVEL 0

this will produce a warning "macro _ITERATOR_DEBUG_LEVEL is redefined"

> +#endif /* _ITERATOR_DEBUG_LEVEL */
> +#endif /* __WIN__*/
> +#endif /* UNIV_DEBUG */

Regards,
Sergei