From: Álvaro Neira Ayuso anayuso@sysmocom.de
This patch provide two new functions for reading and writing from the serial interface. I think this two functions makes it easy reading and writing from the serial interface.
Signed-off-by: Alvaro Neira Ayuso anayuso@sysmocom.de --- v2: Fixed the write function for don't supposing that i will write all the msg in one time. I have studied the limit in the read function and i have used the -errno like the other functions inside serial.c for having consistency. The previous version, i have supposed that we need to liberate the info in case of error, now if we have a error we can check the info that we have received or we must to send again.
include/osmocom/core/serial.h | 4 +++ src/serial.c | 51 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+)
diff --git a/include/osmocom/core/serial.h b/include/osmocom/core/serial.h index 1640a6d..056010b 100644 --- a/include/osmocom/core/serial.h +++ b/include/osmocom/core/serial.h @@ -33,10 +33,14 @@
#include <termios.h>
+struct msgb; + int osmo_serial_init(const char *dev, speed_t baudrate); int osmo_serial_set_baudrate(int fd, speed_t baudrate); int osmo_serial_set_custom_baudrate(int fd, int baudrate); int osmo_serial_clear_custom_baudrate(int fd); +int osmo_serial_write(int fd, struct msgb *msg); +int osmo_serial_read(int fd, struct msgb *msg, int numbytes);
/*! @} */
diff --git a/src/serial.c b/src/serial.c index 66ee756..c71ee6a 100644 --- a/src/serial.c +++ b/src/serial.c @@ -43,6 +43,7 @@ #endif
#include <osmocom/core/serial.h> +#include <osmocom/core/msgb.h>
#if 0 @@ -226,4 +227,54 @@ osmo_serial_clear_custom_baudrate(int fd) return 0; }
+/*! \brief Read datas from Serial interface + * \param[in] fd File descriptor of the open device + * \param[in] msg Struct where we save the info + * \param[in] numbytes Number of bytes that we want read + * \returns 0 for success or negative errno. + */ +int +osmo_serial_read(int fd, struct msgb *msg, int numbytes) +{ + int rc, bread = 0; + + if (numbytes > msg->data_len) + return -errno; + + while (bread < numbytes) { + rc = read(fd, msg->tail, numbytes - bread); + if (rc < 0) + return -errno; + + bread += rc; + msgb_put(msg, rc); + } + + return bread; +} + +/*! \brief Write datas to Serial interface + * \param[in] fd File descriptor of the open device + * \param[in] msg Struct where we have the info for writing + * \returns 0 for success or negative errno. + * + * The msgb->data pointer is updated when consuming this message + */ +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; + + msgb_pull(msg, rc); + bwritten += rc; + } + + return bwritten; +} + /*! @} */
Alvaro Neira wrote:
+++ 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?
+/*! \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.
//Peter
On Wed, Feb 12, 2014 at 07:01:19PM +0100, Peter Stuge wrote:
Alvaro Neira wrote:
+++ 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?
Yes, errno don't return anything here. I'm going to change that with -ENOSPC
- 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.
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.
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?
+/*! \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.
Yes, i'm going to fix that.
Thanks for all Peter. I'm going to send a new version.
-
Alvaro Neira -
Alvaro Neira Ayuso
-
Peter Stuge