| msg104583 - (view) |
Author: Longpoke (Longpoke) |
Date: 2010-04-29 20:30 |
| This function in asyncore is buggy: def _strerror(err): res = os.strerror(err) if res == 'Unknown error': res = errorcode[err] return res - os.strerror may throw ValueError depending on the os, or return a string saying something like: "Unknown error 1234". - os.strerror never returns "Unknown error" for me, so "Unknown error " is always returned for me (Linux 2.6.32) - if os.strerrror failed, it's likely that it wont be in errno.errcode either Maybe it should be written like this: def _strerror(err): try: return strerror(err) except ValueError: return "Unknown error {0}".format(err) |
|
|
| msg104593 - (view) |
Author: Giampaolo Rodola' (giampaolo.rodola) *  |
Date: 2010-04-29 21:24 |
| Good catch. I modified your patch a little bit including a catch for OverflowError exception and a last attempt to look up into errno.errorcode: def _strerror(err): try: return strerror(err) except (ValueError, OverflowError): if err in errorcode: return errorcode[err] return "Unknown error %s" %err Josiah, what do you think? |
|
|
| msg105059 - (view) |
Author: Éric Araujo (eric.araujo) * |
Date: 2010-05-05 17:05 |
| Don’t want to bikeshed here, but why didn’t you keep the newer “{}” string formatting? Module consistency? |
|
|
| msg105157 - (view) |
Author: Giampaolo Rodola' (giampaolo.rodola) * |
Date: 2010-05-06 18:53 |
| Yes, I think it's better to remain consistent with the rest of the module which uses %s all around the place, also for backward compatibility in case someone wants to copy asyncore.py shipped with recent python versions and use it in their code using older python versions. This practice is not so uncommon since asyncore.py received serious bug-fixing only starting from python 2.6. Fixed in r80875 (2.7), r80876 (3.2), r80878 (2.6) and r80879 (3.1) which also includes changes to fix issue 8483. |
|
|
| msg105998 - (view) |
Author: Éric Araujo (eric.araujo) * |
Date: 2010-05-18 19:08 |
| I’ve just checked the diff and the current trunk version of the file, and the change from “os.strerror(err)” to “strerror(err)” seems buggy to me. |
|
|
| msg106002 - (view) |
Author: Longpoke (Longpoke) |
Date: 2010-05-18 19:43 |
| Yes, it should definately be os.sterror. Dunno how I ended up omitting that, sorry. |
|
|
| msg106005 - (view) |
Author: Giampaolo Rodola' (giampaolo.rodola) * |
Date: 2010-05-18 20:19 |
| It slipped under my radar as well. Thanks. Fixed in r81294 (trunk), r81298 (2.6), r81299 (3.2) and r81300 (3.1) which also add tests and include NameError in the list of possible exceptions in case os.strerror() is not supported on the current platform (e.g. Windows CE) in which case "unknown error numbernumbernumber" is returned. |
|
|