trafficserver-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Miles Libbey (JIRA)" <j...@apache.org>
Subject [jira] Created: (TS-301) Change strerror() to strerror_r()
Date Mon, 19 Apr 2010 19:09:50 GMT
Change strerror() to strerror_r()
---------------------------------

                 Key: TS-301
                 URL: https://issues.apache.org/jira/browse/TS-301
             Project: Traffic Server
          Issue Type: Improvement
            Reporter: Miles Libbey


(moving from yahoo bug 520229 to Jira)

In a few places, we use strerror() instead of strerror_r(). The latter is
obviously much better in a threaded application like this.
		
Comment 1
 by Vladimir Legalov  3 years ago at 2006-08-15 09:52:07


Since strerror_r changes errno, all changes must be done very carefully with counting such
"unexpected"
change inside calling code. Please see man for more details.

		

 

Comment 2
 by Ryan Troll 3 years ago at 2006-08-15 13:12:12


Good catch.  You're right, differences like this have to be accounted for.

Does TS actually use the global errno?  I didn't think it's use was
threadsafe.

It's surprising that strerror_r set's errno on Linux -- it doesn't do
that on FreeBSD.  And it's not like it sets the value to anything
besides EINVAL...

-R
	

Comment 3
 by Vladimir Legalov  3 years ago at 2006-08-15 14:16:42


TS is using strerror everywhere and in 99% cases it is not thread-safe 
(except fatal error case).
It works well only because of very low probability of error conditions 
when strerror is called.
-v:)


 

Comment 4
 by Leif Hedstrom  3 years ago at 2006-08-16 07:53:42


But this is only a problem if sterror_r() is called with an invalid errnum, right? What are
the odds of that happening?
We could maybe produce an error in the log if that happens, because if it does, I think it'd
indicate a problem
somewhere in our code.

		

Comment 5
 by Vladimir Legalov  3 years ago at 2006-11-01 14:02:07

strerror_r replacement should be done only per module base. Since strerror_r is changing errno
it is not safe to do
such replacement 'blindly'. I will keep this bug 'open' just as reminder only.

	
Comment 6
 by Leif Hedstrom  3 years ago at 2006-11-01 15:18:26


I still don't understand this, strerror and sterror_r has the same logic/behaviour. Both functions
will modify errno if
the call fails, if not, leave it alone. What am I missing?

		


Comment 7
 by Vladimir Legalov  3 years ago at 2006-11-01 16:37:25


The strerror_r() function could potentially change errno on failure. It is almost impossible
situation in Linux since
strerror_r blindly ignores any
incorrect arguments and calls strerror. However, I think we should not count
on this Linux specific feature. That's why we should check each strerror usage case and wrapping
code for 'errno'
validition. If wrapping code is not requiring original errno after strerror call we can do
strerror_r replacement
blindly.

		

Comment 8
 by Leif Hedstrom  3 years ago at 2006-11-01 16:43:19


Right, but that's the exact same behaviour as sterror, from the man page:

The value of errno is not changed for a successful call, and is set to a nonzero value upon
error.


As far as I can tell, the behaviour is identical for both calls, so changing to strerror_r()
would have no different
side effects from strerror(). Besides, I think the odds of triggering a change to errno is
from calling either of these
functions is very, very small (and would indicate horrible coding on our part).


-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


Mime
View raw message