13 Feb
2014
13 Feb
'14
12:48 p.m.
On Wed, Feb 12, 2014 at 07:01:19PM +0100, Peter Stuge wrote:
We don't lose any data that we have readed. In all the iteration, with
the function msg_put we add inside of msg the interval of data that we
have readed and we update the msg->len.
Alvaro Neira wrote:
Yes, errno don't return anything here. I'm going to change that with
-ENOSPC
+++ b/src/serial.c
..
+/*! \brief Read datas from Serial interface
Minor language comment: "datum" is singular, "data" is plural
+int
+osmo_serial_read(int fd, struct msgb *msg, int numbytes)
+{
+ int rc, bread = 0;
+
+ if (numbytes > msg->data_len)
+ return -errno;
This is a pretty serious bug. What is the value of errno here? + while (bread < numbytes) {
+ rc = read(fd, msg->tail, numbytes - bread);
+ if (rc < 0)
+ return -errno;
Please confirm that any already-received bytes should indeed be
discarded by this function if an error occurs? It may be useful
to document that data loss is intentional in that case.
And what about when read() returns 0, indicating end of file?
(See: man 2 read)
Oh, and one more thing: Shouldn't msg->len be updated?
Yes, i'm going to fix that.
Thanks for all Peter. I'm going to send a new version.
+/*! \brief Write datas to Serial interface
Same minor comment on "data"
+int
+osmo_serial_write(int fd, struct msgb *msg)
+{
+ int rc, bwritten = 0;
+
+ while (msg->len > 0) {
+ rc = write(fd, msg->data, msg->len);
+ if (rc == -1)
+ return -errno;
Another small thing but I think it would be nice if the code is as
consistent as possible between the _read and _write functions, so
rather than _read checking rc < 0 and _write checking rc == -1 have
the code check for errors exactly the same way in both places.