Hacker Newsnew | past | comments | ask | show | jobs | submitlogin

It's wrong (and coreutils get it right) but I don't see why it would have to be deliberate. It could easily just not occur to someone that the code needs to be tested with invalid options, or that it needs to handle invalid options by aborting rather than ignoring. (That in turn would depend on the crate they're using for argument parsing, I imagine.)


Could parsing the `-r` be added without noticing it somehow?

If it was added in bulk, with many other still unsupported option names, why does the program not crash loudly if any such option is used?

A fencepost error is a bug. A double-free is a bug. Accepting an unsupported option and silently ignoring it is not, it takes a deliberate and obviously wrong action.


At least from what I can find, here's the original version of the changed snippet [0]:

    let date_source = if let Some(date) = matches.value_of(OPT_DATE) {
        DateSource::Custom(date.into())
    } else if let Some(file) = matches.value_of(OPT_FILE) {
        DateSource::File(file.into())
    } else {
        DateSource::Now
    };
And after `-r` support was added (among other changes) [1]:

    let date_source = if let Some(date) = matches.get_one::<String>(OPT_DATE) {
        DateSource::Human(date.into())
    } else if let Some(file) = matches.get_one::<String>(OPT_FILE) {
        match file.as_ref() {
            "-" => DateSource::Stdin,
            _ => DateSource::File(file.into()),
        }
    } else if let Some(file) = matches.get_one::<String>(OPT_REFERENCE) {
        DateSource::FileMtime(file.into())
    } else {
        DateSource::Now
    };
Still the same fallback. Not sure one can discern from just looking at the code (and without knowing more about the context, in my case) whether the choice of fallback was intentional and handling the flag was forgotten about.

[0]: https://github.com/yuankunzhang/coreutils/commit/850bd9c32d9...

[1]: https://github.com/yuankunzhang/coreutils/blob/88a7fa7adfa04...


> Accepting an unsupported option and silently ignoring it is not, it takes a deliberate and obviously wrong action.

No, it doesn't. For example, you could have code that recognizes that something "is an option", and silently discards anything that isn't on the recognized list.


> silently discards anything that isn't on the recognized list.

That's a deliberate action.




Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: