[Asterisk-code-review] addons/chan mobile: do not use strerror r (asterisk[13])

[Anonymous Coward]

Anonymous Coward #1000019 has submitted this change and it was merged. ( https://gerrit.asterisk.org/4486 )

Change subject: addons/chan_mobile: do not use strerror_r
......................................................................


addons/chan_mobile: do not use strerror_r

The two reasons why it might be used are that some systems do not
implement strerror in thread safe manner, and that strerror_r returns
the error code in the string in case there's no error message.

However, all of asterisk elsewhere uses strerror() and assumes it
to be thread safe. And in chan_mobile the errno is also explicitly
printed so neither of the above reasons are valid.

The reasoning to remove usage is that there are actually two versions
of strerror_r: XSI and GNU. They are incompatible in their return
value, and there's no easy way to figure out which one is being
used. glibc gives you the GNU version if _GNU_SOURCE is defined,
but the same feature test macro is needed for other symbols. On
all other systems you assumedly get XSI symbol, and compilation warnings
as well as non-working error printing.

Thus the easiest solution is to just remove strerror_r and use
strerror as rest of the code. Alternative is to introduce ast_strerror
in separate translation unit so it can request the XSI symbol in
glibc case, and replace all usage of strerror.

Change-Id: I84d35225b5642d85d48bc35fdf399afbae28a91d
---
M addons/chan_mobile.c
1 file changed, 3 insertions(+), 10 deletions(-)

Approvals:
  Richard Mudgett: Looks good to me, approved
  Anonymous Coward #1000019: Verified
  Joshua Colp: Looks good to me, but someone else must approve



diff --git a/addons/chan_mobile.c b/addons/chan_mobile.c
index f748f79..7077704 100644
--- a/addons/chan_mobile.c
+++ b/addons/chan_mobile.c
@@ -3855,10 +3855,7 @@
 		}
 
 		if ((at_msg = at_read_full(hfp->rsock, buf, sizeof(buf))) < 0) {
-			/* XXX gnu specific strerror_r is assummed here, this
-			 * is not really safe.  See the strerror(3) man page
-			 * for more info. */
-			ast_debug(1, "[%s] error reading from device: %s (%d)\n", pvt->id, strerror_r(errno, buf, sizeof(buf)), errno);
+			ast_debug(1, "[%s] error reading from device: %s (%d)\n", pvt->id, strerror(errno), errno);
 			break;
 		}
 
@@ -3995,7 +3992,7 @@
 			ast_debug(1, "[%s] error parsing message\n", pvt->id);
 			goto e_cleanup;
 		case AT_READ_ERROR:
-			ast_debug(1, "[%s] error reading from device: %s (%d)\n", pvt->id, strerror_r(errno, buf, sizeof(buf)), errno);
+			ast_debug(1, "[%s] error reading from device: %s (%d)\n", pvt->id, strerror(errno), errno);
 			goto e_cleanup;
 		default:
 			break;
@@ -4073,11 +4070,7 @@
 			continue;
 
 		if ((at_msg = at_read_full(pvt->rfcomm_socket, buf, sizeof(buf))) < 0) {
-			if (strerror_r(errno, buf, sizeof(buf)))
-				ast_debug(1, "[%s] error reading from device\n", pvt->id);
-			else
-				ast_debug(1, "[%s] error reading from device: %s (%d)\n", pvt->id, buf, errno);
-
+			ast_debug(1, "[%s] error reading from device: %s (%d)\n", pvt->id, strerror(errno), errno);
 			goto e_cleanup;
 		}
 		ast_debug(1, "[%s] %s\n", pvt->id, buf);

-- 
To view, visit https://gerrit.asterisk.org/4486
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I84d35225b5642d85d48bc35fdf399afbae28a91d
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: Timo Teräs <timo.teras at iki.fi>