This is merely a historical archive of years 2008-2021, before the migration to mailman3.
A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/OpenBSC@lists.osmocom.org/.
Peter Stuge peter at stuge.seAlvaro 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