range error compiler warnings on ARM, comparing char to -1

[Alan]

I tried compiling Ledger on an ARM processor, where the bare `char` type defaults to unsigned. I got warnings at a handful of lines in a couple of source files:

• query.cc
• times.cc
• token.cc

"warning: comparison is always false due to limited range of data type [-Wtype-limits]"

The bug is fairly obvious, and I would be happy to contribute a clean-up fix. But I wanted to ask first whether it would be welcome, because I assume I couldn't possibly be the first person to have encountered this.

Actually most, if not all, of the code isn't even really in play, IINM: it's dead/unreachable and/or redundant/irrelevant. So my proposed fix would be to just excise it.

Or am I missing something?

[Norman Baatz]

I’m getting this too. Is this an issue when running Ledger on ARM?

[Alan Bram]

Sorry for the late reply: this was languishing in my "spam" category, which I unfortunately do not look at often enough.

Yes, this occurs on ARM, as I mentioned. I believe it is safe to ignore (meaning, I believe it does not lead to incorrect calculations in Ledger), but IMHO it should definitely be cleaned up. I offered to fix it, but received no response.

--

---
You received this message because you are subscribed to a topic in the Google Groups "Ledger" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/ledger-cli/bd8v3wiU6oo/unsubscribe.
To unsubscribe from this group and all its topics, send an email to ledger-cli+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/ledger-cli/3adce81c-454d-4279-a9e2-4851cd38ccd8n%40googlegroups.com.

[Martin Michlmayr]

Alan, it would be great if you could submit a pull request through
GitHub.

* Alan Bram <alan...@cornell.edu> [2021-08-18 12:18]:

> Sorry for the late reply: this was languishing in my "spam" category, which
> I unfortunately do not look at often enough.
>
> Yes, this occurs on ARM, as I mentioned. I believe it is safe to ignore
> (meaning, I believe it does not lead to incorrect calculations in Ledger),
> but IMHO it should definitely be cleaned up. I offered to fix it, but
> received no response.
>
> On Mon, Jul 19, 2021 at 4:21 PM Norman Baatz <norman...@gmail.com> wrote:
>
> > I’m getting this too. Is this an issue when running Ledger on ARM?
> >
> > On Wednesday, October 14, 2020 at 11:44:25 PM UTC+2 Alan wrote:
> >
> >> I tried compiling Ledger on an ARM processor, where the bare `char` type
> >> defaults to unsigned. I got warnings at a handful of lines in a couple of
> >> source files:
> >>

> >> - query.cc
> >> - times.cc
> >> - token.cc

> >>
> >> "warning: comparison is always false due to limited range of data type
> >> [-Wtype-limits]"
> >>
> >> The bug is fairly obvious, and I would be happy to contribute a clean-up
> >> fix. But I wanted to ask first whether it would be welcome, because I
> >> assume I couldn't possibly be the first person to have encountered this.
> >>
> >> Actually most, if not all, of the code isn't even really in play, IINM:
> >> it's dead/unreachable and/or redundant/irrelevant. So my proposed fix would
> >> be to just excise it.
> >>
> >> Or am I missing something?
> >>
> > --
> >
> > ---
> > You received this message because you are subscribed to a topic in the
> > Google Groups "Ledger" group.
> > To unsubscribe from this topic, visit
> > https://groups.google.com/d/topic/ledger-cli/bd8v3wiU6oo/unsubscribe.
> > To unsubscribe from this group and all its topics, send an email to
> > ledger-cli+...@googlegroups.com.
> > To view this discussion on the web visit
> > https://groups.google.com/d/msgid/ledger-cli/3adce81c-454d-4279-a9e2-4851cd38ccd8n%40googlegroups.com

> > <https://groups.google.com/d/msgid/ledger-cli/3adce81c-454d-4279-a9e2-4851cd38ccd8n%40googlegroups.com?utm_medium=email&utm_source=footer>
> > .
> >
>
> --
>
> ---
> You received this message because you are subscribed to the Google Groups "Ledger" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to ledger-cli+...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/ledger-cli/CACHqOS8xmnFyaa8h3OAEFnEbCP9BxDw98EuqEzzmTY9gsHzuMQ%40mail.gmail.com.

--
Martin Michlmayr
https://www.cyrius.com/

[Alan]

Will do.

[Alan]

PR in https://github.com/ledger/ledger/pull/2059

[Alan]

(I added a note to this bug to indicate that it does lead to incorrect/garbage error reporting in some cases.)