Panic when reading process comm values with invalid UTF-8 characters

[edigaryev]

comm values can contain pretty much any characters, which breaks /proc/[pid]/stat parsing:

$ echo -e "\xFF" > /proc/$$/comm
$ cargo run --example dump -- $$
    Finished dev [unoptimized + debuginfo] target(s) in 0.01s
     Running `target/debug/examples/dump 15815`
Info for pid=15815
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Incomplete(Some("/proc/15815/stat"))', src/libcore/result.rs:1084:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace.

[eminence]

Thanks for the report. What would you recommend here? I suppose these fields could be converted from String to Vec<u8>, but I'm hesitant to stop using the String type, since it's very convenient. Would using String::from_utf8_lossy() be acceptable for your use-case?

[edigaryev]

Would using String::from_utf8_lossy() be acceptable for your use-case?

That'd work for me. At least it won't panic, which is kinda surprising when using a library, but I guess this way more bugs can be unveiled because people will actually report their panic()'s.

[eminence]

You're right that a library that panics is a little unusual. It was a deliberate choice on my part as a way to encourage bug reports. I don't know if it was the right choice, though. It has proven useful, at the expense of annoying users. Now that I think about it some more, maybe I could introduce a feature that would suppress panics (and turn them into Result::Err(..) instead).

[idanski]

@eminence I use your library in a project, and I'm also concerned about it panicking in production (not so much annoy but a production deal-breaker).

Maybe having an error type for unexpected data/format (with backtrace) - which your users can report - will be a good middle ground?

I will gladly help turn the package panic free if you decide to go for it.

[eminence]

Thanks for the feedback. I've opened a new issue to discuss the general topic of removing as many panics as possible.

I also realized that I've failed to address the original issue brought up by @edigaryev -- I'll plan on doing that work this week.