Change in ...osmo-bts[master]: bts-trx: Log TRXC and TRXD socket recv()/send() failures

2019-10-09 Thread pespin 
pespin has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/c/osmo-bts/+/15702 )

Change subject: bts-trx: Log TRXC and TRXD socket recv()/send() failures
..

bts-trx: Log TRXC and TRXD socket recv()/send() failures

Related: OS#4215
Change-Id: Ic3e41d82b43459495d45873d612a3bd349ac174a
---
M src/osmo-bts-trx/trx_if.c
1 file changed, 23 insertions(+), 5 deletions(-)

Approvals:
  laforge: Looks good to me, but someone else must approve
  osmith: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/src/osmo-bts-trx/trx_if.c b/src/osmo-bts-trx/trx_if.c
index 9933109..1b8ea45 100644
--- a/src/osmo-bts-trx/trx_if.c
+++ b/src/osmo-bts-trx/trx_if.c
@@ -94,14 +94,18 @@
struct phy_link *plink = ofd->data;
struct phy_instance *pinst = phy_instance_by_num(plink, 0);
char buf[1500];
-   int len;
+   ssize_t len;
uint32_t fn;

OSMO_ASSERT(pinst);

len = recv(ofd->fd, buf, sizeof(buf) - 1, 0);
-   if (len <= 0)
+   if (len <= 0) {
+   strerror_r(errno, (char *)buf, sizeof(buf));
+   LOGPPHI(pinst, DTRX, LOGL_ERROR,
+   "recv() failed on TRXD with rc=%zd (%s)\n", len, buf);
return len;
+   }
buf[len] = '\0';

if (!!strncmp(buf, "IND CLOCK ", 10)) {
@@ -144,6 +148,7 @@
struct trx_ctrl_msg *tcm;
char buf[1500];
int len;
+   ssize_t snd_len;

/* get first command */
if (llist_empty(>trx_ctrl_list))
@@ -155,7 +160,12 @@

LOGPPHI(l1h->phy_inst, DTRX, LOGL_DEBUG, "Sending control '%s'\n", buf);
/* send command */
-   send(l1h->trx_ofd_ctrl.fd, buf, len+1, 0);
+   snd_len = send(l1h->trx_ofd_ctrl.fd, buf, len+1, 0);
+   if (snd_len <= 0) {
+   strerror_r(errno, (char *)buf, sizeof(buf));
+   LOGPPHI(l1h->phy_inst, DTRX, LOGL_ERROR,
+   "send() failed on TRXC with rc=%zd (%s)\n", snd_len, 
buf);
+   }

/* start timer */
osmo_timer_schedule(>trx_ctrl_timer, 2, 0);
@@ -979,8 +989,9 @@

buf_len = recv(ofd->fd, buf, sizeof(buf), 0);
if (buf_len <= 0) {
+   strerror_r(errno, (char *)buf, sizeof(buf));
LOGPPHI(l1h->phy_inst, DTRX, LOGL_ERROR,
-   "recv() failed on TRXD with rc=%zd\n", buf_len);
+   "recv() failed on TRXD with rc=%zd (%s)\n", buf_len, 
buf);
return buf_len;
}

@@ -1058,6 +1069,7 @@
 int trx_if_send_burst(struct trx_l1h *l1h, uint8_t tn, uint32_t fn, uint8_t 
pwr,
const ubit_t *bits, uint16_t nbits)
 {
+   ssize_t snd_len;
uint8_t hdr_ver = l1h->config.trxd_hdr_ver_use;
uint8_t buf[TRX_DATA_MSG_MAX_LEN];

@@ -1094,7 +1106,13 @@

/* we must be sure that TRX is on */
if (trx_if_powered(l1h)) {
-   send(l1h->trx_ofd_data.fd, buf, nbits + 6, 0);
+   snd_len = send(l1h->trx_ofd_data.fd, buf, nbits + 6, 0);
+   if (snd_len <= 0) {
+   strerror_r(errno, (char *)buf, sizeof(buf));
+   LOGPPHI(l1h->phy_inst, DTRX, LOGL_ERROR,
+   "send() failed on TRXD with rc=%zd (%s)\n", 
snd_len, buf);
+   return -2;
+   }
} else
LOGPPHI(l1h->phy_inst, DTRX, LOGL_ERROR, "Ignoring TX data, 
transceiver powered off.\n");


--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/15702
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: Ic3e41d82b43459495d45873d612a3bd349ac174a
Gerrit-Change-Number: 15702
Gerrit-PatchSet: 3
Gerrit-Owner: pespin 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria 
Gerrit-Reviewer: laforge 
Gerrit-Reviewer: osmith 
Gerrit-Reviewer: pespin 
Gerrit-MessageType: merged

Change in ...osmo-bts[master]: bts-trx: Log TRXC and TRXD socket recv()/send() failures

2019-10-09 Thread osmith 
osmith has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-bts/+/15702 )

Change subject: bts-trx: Log TRXC and TRXD socket recv()/send() failures
..


Patch Set 2: Code-Review+2


--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/15702
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: Ic3e41d82b43459495d45873d612a3bd349ac174a
Gerrit-Change-Number: 15702
Gerrit-PatchSet: 2
Gerrit-Owner: pespin 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria 
Gerrit-Reviewer: laforge 
Gerrit-Reviewer: osmith 
Gerrit-Reviewer: pespin 
Gerrit-Comment-Date: Wed, 09 Oct 2019 08:20:22 +
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

Change in ...osmo-bts[master]: bts-trx: Log TRXC and TRXD socket recv()/send() failures

2019-10-08 Thread laforge 
laforge has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-bts/+/15702 )

Change subject: bts-trx: Log TRXC and TRXD socket recv()/send() failures
..


Patch Set 2: Code-Review+1


--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/15702
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: Ic3e41d82b43459495d45873d612a3bd349ac174a
Gerrit-Change-Number: 15702
Gerrit-PatchSet: 2
Gerrit-Owner: pespin 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria 
Gerrit-Reviewer: laforge 
Gerrit-Reviewer: pespin 
Gerrit-Comment-Date: Tue, 08 Oct 2019 20:33:39 +
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

Change in ...osmo-bts[master]: bts-trx: Log TRXC and TRXD socket recv()/send() failures

2019-10-08 Thread pespin 
Hello fixeria, Jenkins Builder,

I'd like you to reexamine a change. Please visit

https://gerrit.osmocom.org/c/osmo-bts/+/15702

to look at the new patch set (#2).

Change subject: bts-trx: Log TRXC and TRXD socket recv()/send() failures
..

bts-trx: Log TRXC and TRXD socket recv()/send() failures

Related: OS#4215
Change-Id: Ic3e41d82b43459495d45873d612a3bd349ac174a
---
M src/osmo-bts-trx/trx_if.c
1 file changed, 23 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/02/15702/2
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/15702
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: Ic3e41d82b43459495d45873d612a3bd349ac174a
Gerrit-Change-Number: 15702
Gerrit-PatchSet: 2
Gerrit-Owner: pespin 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria 
Gerrit-Reviewer: pespin 
Gerrit-MessageType: newpatchset

Change in ...osmo-bts[master]: bts-trx: Log TRXC and TRXD socket recv()/send() failures

2019-10-08 Thread pespin 
pespin has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-bts/+/15702 )

Change subject: bts-trx: Log TRXC and TRXD socket recv()/send() failures
..


Patch Set 1:

(1 comment)

https://gerrit.osmocom.org/#/c/15702/1/src/osmo-bts-trx/trx_if.c
File src/osmo-bts-trx/trx_if.c:

https://gerrit.osmocom.org/#/c/15702/1/src/osmo-bts-trx/trx_if.c@104
PS1, Line 104: strerror_r
> I prefer using stack-buffer functions, it's less prone to errors in the 
> future.
For instance, I bet strerror is not signal-safe :)



--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/15702
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: Ic3e41d82b43459495d45873d612a3bd349ac174a
Gerrit-Change-Number: 15702
Gerrit-PatchSet: 1
Gerrit-Owner: pespin 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria 
Gerrit-Reviewer: pespin 
Gerrit-Comment-Date: Tue, 08 Oct 2019 13:53:33 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin 
Comment-In-Reply-To: fixeria 
Gerrit-MessageType: comment

Change in ...osmo-bts[master]: bts-trx: Log TRXC and TRXD socket recv()/send() failures

2019-10-08 Thread pespin 
pespin has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-bts/+/15702 )

Change subject: bts-trx: Log TRXC and TRXD socket recv()/send() failures
..


Patch Set 1:

(1 comment)

https://gerrit.osmocom.org/#/c/15702/1/src/osmo-bts-trx/trx_if.c
File src/osmo-bts-trx/trx_if.c:

https://gerrit.osmocom.org/#/c/15702/1/src/osmo-bts-trx/trx_if.c@104
PS1, Line 104: strerror_r
> This is a single thread application, so calling strerror() would be enough.
I prefer using stack-buffer functions, it's less prone to errors in the future.



--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/15702
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: Ic3e41d82b43459495d45873d612a3bd349ac174a
Gerrit-Change-Number: 15702
Gerrit-PatchSet: 1
Gerrit-Owner: pespin 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria 
Gerrit-Reviewer: pespin 
Gerrit-Comment-Date: Tue, 08 Oct 2019 13:52:27 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: fixeria 
Gerrit-MessageType: comment

Change in ...osmo-bts[master]: bts-trx: Log TRXC and TRXD socket recv()/send() failures

2019-10-08 Thread fixeria 
fixeria has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-bts/+/15702 )

Change subject: bts-trx: Log TRXC and TRXD socket recv()/send() failures
..


Patch Set 1: Code-Review+1

(5 comments)

Checking rc of send() - cool! Coverity was complaining about that for a long 
time. I don't like using goto without the real need for that, otherwise I would 
CR+2. Also, I would prefer strerror().

https://gerrit.osmocom.org/#/c/15702/1/src/osmo-bts-trx/trx_if.c
File src/osmo-bts-trx/trx_if.c:

https://gerrit.osmocom.org/#/c/15702/1/src/osmo-bts-trx/trx_if.c@104
PS1, Line 104: strerror_r
This is a single thread application, so calling strerror() would be enough.


https://gerrit.osmocom.org/#/c/15702/1/src/osmo-bts-trx/trx_if.c@165
PS1, Line 165: strerror_r
Same.


https://gerrit.osmocom.org/#/c/15702/1/src/osmo-bts-trx/trx_if.c@992
PS1, Line 992: strerror_r
Same.


https://gerrit.osmocom.org/#/c/15702/1/src/osmo-bts-trx/trx_if.c@
PS1, Line : goto
Any reason to use goto here? This is the only failure branch right now, and I 
don't think there will be more calls to send() in this function.


https://gerrit.osmocom.org/#/c/15702/1/src/osmo-bts-trx/trx_if.c@1118
PS1, Line 1118: strerror_r
Same.



--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/15702
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: Ic3e41d82b43459495d45873d612a3bd349ac174a
Gerrit-Change-Number: 15702
Gerrit-PatchSet: 1
Gerrit-Owner: pespin 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria 
Gerrit-Comment-Date: Tue, 08 Oct 2019 13:49:24 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

Change in ...osmo-bts[master]: bts-trx: Log TRXC and TRXD socket recv()/send() failures

2019-10-08 Thread pespin 
pespin has uploaded this change for review. ( 
https://gerrit.osmocom.org/c/osmo-bts/+/15702


Change subject: bts-trx: Log TRXC and TRXD socket recv()/send() failures
..

bts-trx: Log TRXC and TRXD socket recv()/send() failures

Related: OS#4215
Change-Id: Ic3e41d82b43459495d45873d612a3bd349ac174a
---
M src/osmo-bts-trx/trx_if.c
1 file changed, 25 insertions(+), 5 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/02/15702/1

diff --git a/src/osmo-bts-trx/trx_if.c b/src/osmo-bts-trx/trx_if.c
index 9933109..af64ed6 100644
--- a/src/osmo-bts-trx/trx_if.c
+++ b/src/osmo-bts-trx/trx_if.c
@@ -94,14 +94,18 @@
struct phy_link *plink = ofd->data;
struct phy_instance *pinst = phy_instance_by_num(plink, 0);
char buf[1500];
-   int len;
+   ssize_t len;
uint32_t fn;

OSMO_ASSERT(pinst);

len = recv(ofd->fd, buf, sizeof(buf) - 1, 0);
-   if (len <= 0)
+   if (len <= 0) {
+   strerror_r(errno, (char *)buf, sizeof(buf));
+   LOGPPHI(pinst, DTRX, LOGL_ERROR,
+   "recv() failed on TRXD with rc=%zd (%s)\n", len, buf);
return len;
+   }
buf[len] = '\0';

if (!!strncmp(buf, "IND CLOCK ", 10)) {
@@ -144,6 +148,7 @@
struct trx_ctrl_msg *tcm;
char buf[1500];
int len;
+   ssize_t snd_len;

/* get first command */
if (llist_empty(>trx_ctrl_list))
@@ -155,7 +160,12 @@

LOGPPHI(l1h->phy_inst, DTRX, LOGL_DEBUG, "Sending control '%s'\n", buf);
/* send command */
-   send(l1h->trx_ofd_ctrl.fd, buf, len+1, 0);
+   snd_len = send(l1h->trx_ofd_ctrl.fd, buf, len+1, 0);
+   if (snd_len <= 0) {
+   strerror_r(errno, (char *)buf, sizeof(buf));
+   LOGPPHI(l1h->phy_inst, DTRX, LOGL_ERROR,
+   "send() failed on TRXC with rc=%zd (%s)\n", snd_len, 
buf);
+   }

/* start timer */
osmo_timer_schedule(>trx_ctrl_timer, 2, 0);
@@ -979,8 +989,9 @@

buf_len = recv(ofd->fd, buf, sizeof(buf), 0);
if (buf_len <= 0) {
+   strerror_r(errno, (char *)buf, sizeof(buf));
LOGPPHI(l1h->phy_inst, DTRX, LOGL_ERROR,
-   "recv() failed on TRXD with rc=%zd\n", buf_len);
+   "recv() failed on TRXD with rc=%zd (%s)\n", buf_len, 
buf);
return buf_len;
}

@@ -1058,6 +1069,7 @@
 int trx_if_send_burst(struct trx_l1h *l1h, uint8_t tn, uint32_t fn, uint8_t 
pwr,
const ubit_t *bits, uint16_t nbits)
 {
+   ssize_t snd_len;
uint8_t hdr_ver = l1h->config.trxd_hdr_ver_use;
uint8_t buf[TRX_DATA_MSG_MAX_LEN];

@@ -1094,11 +1106,19 @@

/* we must be sure that TRX is on */
if (trx_if_powered(l1h)) {
-   send(l1h->trx_ofd_data.fd, buf, nbits + 6, 0);
+   snd_len = send(l1h->trx_ofd_data.fd, buf, nbits + 6, 0);
+   if (snd_len <= 0)
+   goto snd_err;
} else
LOGPPHI(l1h->phy_inst, DTRX, LOGL_ERROR, "Ignoring TX data, 
transceiver powered off.\n");

return 0;
+
+snd_err:
+   strerror_r(errno, (char *)buf, sizeof(buf));
+   LOGPPHI(l1h->phy_inst, DTRX, LOGL_ERROR,
+   "send() failed on TRXD with rc=%zd (%s)\n", snd_len, buf);
+   return -2;
 }



--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/15702
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: Ic3e41d82b43459495d45873d612a3bd349ac174a
Gerrit-Change-Number: 15702
Gerrit-PatchSet: 1
Gerrit-Owner: pespin 
Gerrit-MessageType: newchange