Attention is currently required from: Stanislav Ponomarev.

View Change

10 comments:

• Commit Message:

  • Patch Set #2, Line 9: pet

    maybe remove "pet" :)

  • Patch Set #2, Line 9: I was working on an

    During the development of

  • Patch Set #2, Line 10: when i implemented

    implementation of

  • Patch Set #2, Line 11: i

    uppercase

  • Patch Set #2, Line 14: I added

    This patch adds

  • Patch Set #2, Line 18: After that

    TESTED=

• Patchset:

  • Patch Set #2:

    Thank you for you patch! I added some comments.

    I really like your original commit message, it explains everything very well. But I added few comments on it, just because typically commit message is written is more formal style (since it stays in git history forever).

    Also when I got an email about this patch, it does not have display name (literally email from "Name of user not set"). I think it's probably display name or full name in Gerrit account is empty, or both. You can set it to anything, any nickname is fine.

• File serial.c:

  • Patch Set #2, Line 380:

    int ret = tcflush(sp_fd, TCIFLUSH);
    	if (ret == 0)
    		return;
    

    We typically use more compact variation. I don't see this exact ret value is used again, so you can do like this:

    	if (!tcflush(sp_fd, TCIFLUSH))
    		return;

  • Patch Set #2, Line 392: ret > 0

    A question about return codes: `serialport_read_nonblock` can return positive or negative error codes, I am wondering does positive return code reliably indicates that no more data available to read? I just read through the comment and the code of `serialport_read_nonblock` and I can't decide on that :)

    If it works reliably, maybe you can add a bit longer comment, like "Positive error code indicates no more data available, which is normal situation and means flush completed successfully".

  • Patch Set #2, Line 401: return;

    I am thinking, maybe change slightly with less returns. It's harder to read when every second line returns.

      if (errno == ENOTTY) {
       <... serialport_read_nonblock loop ...>
       if (ret < 0)
        msg_perr("Could not flush serial port incoming buffer: read has failed");
      } else {
       msg_perr_strerror("Could not flush serial port incoming buffer: ");
      }

To view, visit change 79112. To unsubscribe, or for help writing mail filters, visit settings.