This is an archive of the discontinued Mercurial Phabricator instance.

dispatch: validate return type from dispatch() return value
Needs RevisionPublic

Authored by indygreg on May 6 2018, 12:11 AM.

Details

Reviewers
admin
durin42
Group Reviewers
hg-reviewers
Summary

This commit adds type checking to the result of dispatch() so
we catch non-None and non-int values. This prevents crashes
when we later try to "& 255" this value.

The Python 3 test failures introduced previously have all gone away.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

indygreg created this revision.May 6 2018, 12:11 AM
yuja added a subscriber: yuja.May 6 2018, 8:29 AM

IIRC, SystemExit is caught at callcatch and translated to a status code.

+ # The logic here attempts to mimic how CPython's pythonrun.c does things.
+ # Essentially:
+ #
+ # * Exit is controlled by returning a value or raising SystemExit.
+ # * An integer value exits with that exit code.
+ # * A value of `None is interpreted as 0`.
+ # * A non-integer, non-None value results in that value being printed
+ # and an exit code of `1`.
+ # * SystemExit can have a `code` attribute containing the exit value.

try:
  • status = (dispatch(req) or 0)

+ code = dispatch(req) or 0

except error.StdioError as e:
    err = e
  • status = -1

+ code = -1
+ except SystemExit as err:
+ try:
+ code = err.code
+ err = None
+ except AttributeError:

indygreg edited the summary of this revision. (Show Details)May 12 2018, 12:48 AM
indygreg retitled this revision from dispatch: shore up exit handling to dispatch: validate return type from dispatch() return value.
indygreg updated this revision to Diff 8654.
admin requested changes to this revision.May 31 2018, 3:10 PM
admin added a subscriber: admin.

This looks good, but needs rebased.

This revision now requires changes to proceed.May 31 2018, 3:10 PM
durin42 requested changes to this revision.May 31 2018, 3:11 PM
durin42 added a subscriber: durin42.

(derp, that was me, but my password vault mis-fired and I got admin instead of me.)