Re: [lmi] expand_html() msw64: 'SIZE_MAX <= index' always false

[Greg Chicares]

On 2019-03-24 17:00, Vadim Zeitlin wrote:
> On Sun, 24 Mar 2019 10:09:21 +0000 Greg Chicares <address@hidden> wrote:
> 
> GC> Vadim--is this the right fix for the problem below?
[...]
> GC> -            auto const index = std::strtoul(s.c_str() + open_pos + 1, 
> &stop, 10);
> GC> +            auto const index = std::strtoull(s.c_str() + open_pos + 1, 
> &stop, 10);
[...]
> GC> /opt/lmi/src/lmi/pdf_command_wx.cpp:278:63: error: comparison is always 
> false due to limited range of data type [-Werror=type-limits]
> GC>              if(stop != s.c_str() + s.length() - 1 || SIZE_MAX <= index)
[...]
>  Sorry, I'm not sure why would we want to use long long here when the
> indices are typically small integers, i.e. should fit in a short if not
> char.

I figured that you had chosen SIZE_MAX because you wanted type std::size_t,
much as we compare std::string::find() results to std::string::npos.

However, replacing the old implicit assumption that size_t is UL by a new
implicit assumption that it's ULL seems to have been a mistake in my patch.
(Of course there exists no std::strto_size_t(), but that's a good thing.)

> The real problem seems to be a rather head-scratching use of SIZE_MAX
> here, to be honest I have absolutely no recollection why did I decide to do
> it (and out Git history doesn't contain the details of my commits in that
> branch, so even if I had written a message explaining it back when I added
> it, it's not available any longer...).

Really? I thought we had taken great care to preserve your commits. Thus,
these two commits are distinct [found by 'git log --all -G'SIZE_MAX'']:

commit a78b402a87d2954de9bc8bf98ecf0ccbe75cb294
Author: Gregory W. Chicares <address@hidden>
Date:   2018-01-27T07:08:11+00:00

    Import remaining new files verbatim from vz-no-xslfo

commit 416ab02fb8b8ea4da036fa86d27fdf19772f6a69
Author: Vadim Zeitlin <address@hidden>
Date:   2017-06-30T01:59:13+00:00

    Add support for vector variables to PDF generating code
    
    Recognize anything of the form "name[index]" as a vector.

Doesn't 'git show 416ab02fb8' display one of your original,
separate commits?

>  Looking at the code anew it seems that we should really be using ULONG_MAX
> instead, which wouldn't generate this warning (I've tested and confirmed
> that this is indeed the case). I.e. I'd rather propose this patch instead:

Committed, thanks.

> Note that I've also changed "<=" into "==", but we could leave "<=" too and
> no warning would still be given when comparing with this constant, which is
> always defined appropriately for the size of "index" itself (i.e. 32 bits
> under MSW, even in 64 bit builds).

Yes, I think '==' is clearer.