Skip to content

src: fix --abort_on_uncaught_exception arg parsing #13004

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom
Merged
merged 1 commit into from
May 16, 2017
Merged

src: fix --abort_on_uncaught_exception arg parsing #13004

merged 1 commit into from
May 16, 2017

Conversation

sam-github
Copy link
Contributor

Fix c0bde73, which inadvertently introduced a use of strcmp() without
correctly comparing its return to zero. Caught by coverity:

    >>>     CID 169223:  Integer handling issues  (CONSTANT_EXPRESSION_RESULT)
    >>>     The "or" condition "strcmp(arg, "--abort-on-uncaught-exception") || strcmp(arg, "--abort_on_uncaught_exception")" will always be true because "arg" cannot be equal to two different values at the same time, so it must be not equal to at least one of them.
    3909         } else if (strcmp(arg, "--abort-on-uncaught-exception") ||
    3910                    strcmp(arg, "--abort_on_uncaught_exception")) {
    3911           abort_on_uncaught_exception = true;
    3912           // Also a V8 option.  Pass through as-is.
    3913           new_v8_argv[new_v8_argc] = arg;
    3914           new_v8_argc += 1;

See: https://github.com/nodejs/node/pull/12892/files#r116055631

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

src

@nodejs-github-bot nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label May 12, 2017
@addaleax addaleax added dont-land-on-v4.x lib / src Issues and PRs related to general changes in the lib or src directory. labels May 12, 2017
@sam-github
Copy link
Contributor Author

ci: https://ci.nodejs.org/job/node-test-pull-request/8047/

@sam-github
src: fix --abort_on_uncaught_exception arg parsing
53dae83 
Fix c0bde73, which inadvertently introduced a use of strcmp() without
correctly comparing its return to zero. Caught by coverity:

        >>>     CID 169223:  Integer handling issues  (CONSTANT_EXPRESSION_RESULT)
        >>>     The "or" condition "strcmp(arg, "--abort-on-uncaught-exception") || strcmp(arg, "--abort_on_uncaught_exception")" will always be true because "arg" cannot be equal to two different values at the same time, so it must be not equal to at least one of them.
        3909         } else if (strcmp(arg, "--abort-on-uncaught-exception") ||
        3910                    strcmp(arg, "--abort_on_uncaught_exception")) {
        3911           abort_on_uncaught_exception = true;
        3912           // Also a V8 option.  Pass through as-is.
        3913           new_v8_argv[new_v8_argc] = arg;
        3914           new_v8_argc += 1;

PR-URL: nodejs#13004
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
@sam-github sam-github force-pushed the fix-abort-on-uncaught-exception branch from 04a7057 to 53dae83 Compare May 16, 2017 21:04
@sam-github sam-github merged commit 53dae83 into nodejs:master May 16, 2017
@sam-github sam-github deleted the fix-abort-on-uncaught-exception branch May 16, 2017 21:07
anchnk pushed a commit to anchnk/node that referenced this pull request May 19, 2017
@sam-github
src: fix --abort_on_uncaught_exception arg parsing
859d7be 
Fix c0bde73, which inadvertently introduced a use of strcmp() without
correctly comparing its return to zero. Caught by coverity:

        >>>     CID 169223:  Integer handling issues  (CONSTANT_EXPRESSION_RESULT)
        >>>     The "or" condition "strcmp(arg, "--abort-on-uncaught-exception") || strcmp(arg, "--abort_on_uncaught_exception")" will always be true because "arg" cannot be equal to two different values at the same time, so it must be not equal to at least one of them.
        3909         } else if (strcmp(arg, "--abort-on-uncaught-exception") ||
        3910                    strcmp(arg, "--abort_on_uncaught_exception")) {
        3911           abort_on_uncaught_exception = true;
        3912           // Also a V8 option.  Pass through as-is.
        3913           new_v8_argv[new_v8_argc] = arg;
        3914           new_v8_argc += 1;

PR-URL: nodejs#13004
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
@jasnell jasnell mentioned this pull request May 28, 2017
Closed
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@jasnell jasnell jasnell approved these changes

@addaleax addaleax addaleax approved these changes

@lpinca lpinca lpinca approved these changes

@cjihrig cjihrig cjihrig approved these changes

@richardlau richardlau richardlau approved these changes

@gibfahn gibfahn gibfahn approved these changes

Assignees
No one assigned
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@sam-github @jasnell @addaleax @lpinca @cjihrig @richardlau @gibfahn @nodejs-github-bot