Nico Huber posted comments on this change.

View Change

Patch set 6:Verified -1Code-Review +2

(3 comments)

• File linux_spi.c:

  • Patch Set #5, Line 143: if (feof(fp))

    In any case, I went ahead and added an feof() to check for this
    condition. I was thinking of using stat(), but that would add a few
    more lines and seemed redundant with fopen() error handling in this
    case. LMK if you have a preference.

    feof() is fine. stat() seems overkill and I'm not sure how well it is
    implemented for sysfs ;)

    
    BTW - It may be worth moving all this logic to a file handling
    library some day, especially as we improve support for Linux
    interfaces that involve searching thru sysfs and proc and
    potentially opening/closing a lot of files.

    Sounds good, especially if we hit the same pattern (e.g. reading an
    unsigned) from a file again.

  • Patch Set #5, Line 154: msg_pwarn("Buffer size %ld from %s seems wrong.\n", tmp, BUF_SIZE_FROM_SYSFS);

    Yeah, that's certainly an assumption that I suppose language lawyers may ta

    Same for the part of the string we didn't read, e.g. what if we
    don't hit EOF after 10 chars oO

  • Patch Set #5, Line 163:

    Should I follow-up with a patch to use sysconf(_SC_PAGESIZE) instead, as su

    Not a big issue, IMO. But if you do, I'm happy to review.

To view, visit change 22344. To unsubscribe, visit settings.