On 8/31/23 07:26, Michael Shavit wrote: > On Thu, Aug 31, 2023 at 6:57 PM Rob Landley <r...@landley.net> wrote: >> Could you send me the test case you hit that motivated this change? (I >> haven't >> got a tests/devmem.test yet and am not 100% sure how I'd even set that up in >> mkroot under qemu, but I should work out something...) > > I ran something like this: > # devmem 0x000000008b300000 8 0x8FFFFFFFFFFFFFFF > # devmem 0x000000008b300000 8 > 0x8fffffffffffffff > > # devmem 0x000000008b300000 8 500k > # devmem 0x000000008b300000 8 > 0x0000000007d000 > > # devmem 0x000000008b300000 8 0xFFFFFFFFFFFFFFFF > # devmem 0x000000008b300000 8 > 0xffffffffffffffff
That would be the one that failed then? > # devmem 0x000000008b300000 4 0xFFFFFFFF > # devmem 0x000000008b300000 4 > 0xffffffff > > # devmem 0x000000008b300000 4 0x1FFFFFFFF > devmem: data: 8589934591 exceeds write width: 4 Which is true... > I don't, but problem is I don't know if other users of devmem have > come to expect and depend on this behavior. Are we ok potentially > breaking compatibility for a bug fix? > >> Is sizeof(unsigned long long) ever _not_ 8? > > IIUC, technically the C spec only guarantees that it's at least 8. Toybox uses LP64: https://landley.net/toybox/design.html#bits Which technically only guarantees that long long is at _least_ 8 bytes, but I have yet to see a system where it's bigger and somebody (the musl-libc maintainer?) brought up several existing things changing it would break when I asked him.... > But > anyways, the intent of using sizeof() was to make sure any updates of > the data/max_value types would propagate to the check. > >> >> I take it you're still ok with the decimal output being signed instead of >> unsigned? > > What do you mean, isn't %llx unsigned? Line 72: printf((!strchr(*toys.optargs, 'x')) ? "%0*lld\n" : "0x%0*llx\n", bytes*2, data); Probably fine, but now input is unsigned and output is signed so I thought I'd ask somebody with domain experience using this command. :) >> How does this look (untested): > > I can test this out in a bit :) . ... >> // DATA? Report out of range values as errors rather than truncating. >> - if (writing) data = atolx_range(toys.optargs[2], 0, (1ULL<<(8*bytes))-1); >> + if (writing && (data = atollu(toys.optargs[2]))>(~0LL)>>(64-8*bytes)) > > Shouldn't it be (~0ULL)? I think (~0LL) would make this an arithmetic > shift instead of a logical shift. I said it was untested. :) Yup, fixed. Rob _______________________________________________ Toybox mailing list Toybox@lists.landley.net http://lists.landley.net/listinfo.cgi/toybox-landley.net
