Does not work when running as 32-bit process on 64-bit machines
h33p opened this issue · 3 comments
When running procfs::process::all_processes() parsing fails in stat.rs, because fields overflow usize. Here's an example error:
.cargo/registry/src/github.com-1ecc6299db9ec823/procfs-0.9.1/src/process/stat.rs:306 (please report this procfs bug)
Changing start_data to be a u64 makes the problem go away for the field, but then it fails to parse the next one:
.cargo/registry/src/github.com-1ecc6299db9ec823/procfs-0.9.1/src/process/stat.rs:307 (please report this procfs bug)
While it is not a fairly common application nowadays, a user could still run into it when downloading the wrong build of the program. I don't know what your view is on this, but this could be solved in at least a couple of ways:
- Just replace all parsable occurances of usize to u64 (there are 60 instances of
usizein total, 26 excludingdiskstatsmodule). - Do 1, but selectively. Ignore certain fields that would never be over 4G.
- Create a typedef that can switch between usize and u64 as a feature. Given the way rust compiles crates, it could prove rather fragile when 2 crates expect different features, but flexible at the same time.
If this change is not worth it I'm still interested whether diskstats should be using usize, because nowadays disks are in terabytes of size, and after long runtime the stats could overflow usize on pure 32-bit machines. However, I'm not sure how Linux does it, and I could be wrong, I'm just assuming the numbers are not limited to native word size on 32-bit machines.
Either way, if it interests you I would be willing to contribute this change, just let me know which direction to go for.
Hi, thanks for opening this issue.
I've done some testing in the past on 32-bit systems, I have to admit I've never tested a 32-bit build of procfs running on a 64-bit system.
I also have to admit, I'm not 100% sure what to do here.
If I'm reading this correctly, the diskstat fields are unsigned long, which are generally 32-bits on 32-bit systems I think. This doesn't mean that the diskstats has to follow, though. Using u64 here seems pretty reasonable.
The issue with start_data is maybe more interesting, as it gets more directly at the root issue: is it reasonable to expect a 32-bit procfs build to work on 64-bit systems? In general, I guess people would probably expect this to work. Another interesting use case is: maybe someone is using procfs to parse data that was generated on another machine. This probably isn't common (and I bet procfs gets some things wrong here), but this would be another case where maybe it makes sense to replace usize with u64.
I also note that some fields are storing some type of address, but are already u64 (not usize), like: https://docs.rs/procfs/0.10.0/procfs/process/struct.Stat.html#structfield.startcode
So already procfs isn't self-consistent.
It seems like diskstats do use native word size, making it the same as addresses. But the point about using procfs to parse other machines' data is also very interesting and could be another valid usecase. In that case, to support these usecaess, changing these values to be u64 would do more good than harm, I believe.
I will create a pull request tomorrow, and will try to keep some values as usize, where it makes sense, but will be able to change the rest to u64 if that's what you'd prefer.