[Toybox] [PATCH] devmem: Fix 8 byte wide writes

enh enh at google.com
Mon Oct 9 08:26:05 PDT 2023


it's a bit early in the morning for me, but doesn't the change from
`unsigned long long` to `unsigned long` mean that by the time you test
the result it can't be too large because you've already truncated?

On Sat, Oct 7, 2023 at 11:39 PM Rob Landley <rob at landley.net> wrote:
>
> On 10/6/23 14:59, enh wrote:
> > On Fri, Oct 6, 2023 at 12:35 PM Michael Shavit via Toybox
> > <toybox at lists.landley.net> wrote:
> >>
> >> On Fri, Oct 6, 2023 at 4:41 AM Rob Landley <rob at landley.net> wrote:
> >> >
> >> > I could adjust _just_ addr to use unsigned long instead of unsigned long long,
> >> > but I don't want two atollu()/atolu() and letting you do a longer address and
> >> > silently truncating it isn't great either. Also, a 32-bit platform can't do an
> >> > atomic 64 bit read/write either (it's gonna do 2 32 bit ones behind the scenes)
> >> > and I kinda don't want to claim it won't? Which means removing the 8 byte option
> >> > when building for 32 bits, which is trivial to do in the code:
> >> >
> >> > +    else if (sizeof(long)==8 && bytes==8) *(unsigned long *)p = data;
> >>
> >>
> >> We can also emit an error earlier in the function if the WIDTH
> >> parameter is 8 on a 32 bit platform.
> >
> >> IMO disallowing 64bit writes is less risky of headaches than silently
> >> non-atomic behavior.  A clear error message might help alleviate the
> >> documentation issue, but I'll agree it's less than ideal to find out
> >> that something won't work after the fact.
> >
> > eh, i actually _prefer_ the error message option --- if someone tries
> > this on a 32-bit device, they're probably confused enough that an
> > error message will be helpful. whereas seeing nothing in the --help
> > text is unlikely to teach them anything.
> >
> > (bruce tognazzini used to argue against graying out menu items/buttons
> > in guis for similar reasons --- "how can you teach the user what
> > they've done wrong in that case?". i've never been annoyed by an
> > explanation of what i'm doing wrong, but i've often been annoyed by a
> > button i can't click and don't understand why!)
>
> Let me know how commit 5f153b56214f works? I don't have a test environment for
> this, but it seems to produce the errors right on 32 and 64 bit at least...
>
> Rob


More information about the Toybox mailing list