Skip to content

Conversation

@sam-github
Copy link
Contributor

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines

gcc 8+ recognizes that space has not been left for the pid and that the
return value of snprintf() isn't checked. Leave a little space for the
pid to prevent `-Wformat-truncation`.
In recent gcc, -Wrestrict warns when an argument passed to a
restrict-qualified parameter aliases with another argument.
@sam-github sam-github added the semver-major PRs that contain breaking changes and should be released in the next major version. label Dec 3, 2018
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. inspector Issues and PRs related to the V8 inspector protocol labels Dec 3, 2018
@sam-github
Copy link
Contributor Author

@addaleax
Copy link
Member

addaleax commented Dec 3, 2018

Can you explain the semver label? It’s not obvious to me :)

void GetHumanReadableProcessName(char (*name)[1024]) {
char title[1024] = "Node.js";
// Leave room after title for pid, which can be up to 20 digits for 64 bit.
char title[1000] = "Node.js";
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@addaleax see ---^ I'm OK with it not being semver-major, but if there was a process-name longer than 1000 chars, it would now be slightly truncated. How paranoid should we be? I'll let someone else decide.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is okay, given that names of that length are very unlikely, and we mostly only use this for debugging purposes?

@sam-github sam-github removed the semver-major PRs that contain breaking changes and should be released in the next major version. label Dec 3, 2018
@sam-github
Copy link
Contributor Author

@addaleax @eugeneo PTAL

@sam-github
Copy link
Contributor Author

Landed in bcef949...c49d87e

@sam-github sam-github closed this Dec 7, 2018
@sam-github sam-github deleted the fix-src-warnings branch December 7, 2018 18:14
sam-github added a commit that referenced this pull request Dec 7, 2018
gcc 8+ recognizes that space has not been left for the pid and that the
return value of snprintf() isn't checked. Leave a little space for the
pid to prevent `-Wformat-truncation`.

PR-URL: #24810
Reviewed-By: Richard Lau <[email protected]>
sam-github added a commit that referenced this pull request Dec 7, 2018
In recent gcc, -Wrestrict warns when an argument passed to a
restrict-qualified parameter aliases with another argument.

PR-URL: #24810
Reviewed-By: Richard Lau <[email protected]>
MylesBorins pushed a commit that referenced this pull request Dec 7, 2018
gcc 8+ recognizes that space has not been left for the pid and that the
return value of snprintf() isn't checked. Leave a little space for the
pid to prevent `-Wformat-truncation`.

PR-URL: #24810
Reviewed-By: Richard Lau <[email protected]>
MylesBorins pushed a commit that referenced this pull request Dec 7, 2018
In recent gcc, -Wrestrict warns when an argument passed to a
restrict-qualified parameter aliases with another argument.

PR-URL: #24810
Reviewed-By: Richard Lau <[email protected]>
@BethGriggs BethGriggs mentioned this pull request Dec 18, 2018
refack pushed a commit to refack/node that referenced this pull request Jan 14, 2019
gcc 8+ recognizes that space has not been left for the pid and that the
return value of snprintf() isn't checked. Leave a little space for the
pid to prevent `-Wformat-truncation`.

PR-URL: nodejs#24810
Reviewed-By: Richard Lau <[email protected]>
refack pushed a commit to refack/node that referenced this pull request Jan 14, 2019
In recent gcc, -Wrestrict warns when an argument passed to a
restrict-qualified parameter aliases with another argument.

PR-URL: nodejs#24810
Reviewed-By: Richard Lau <[email protected]>
BethGriggs pushed a commit that referenced this pull request Feb 12, 2019
gcc 8+ recognizes that space has not been left for the pid and that the
return value of snprintf() isn't checked. Leave a little space for the
pid to prevent `-Wformat-truncation`.

PR-URL: #24810
Reviewed-By: Richard Lau <[email protected]>
BethGriggs pushed a commit that referenced this pull request Feb 12, 2019
In recent gcc, -Wrestrict warns when an argument passed to a
restrict-qualified parameter aliases with another argument.

PR-URL: #24810
Reviewed-By: Richard Lau <[email protected]>
@BethGriggs BethGriggs mentioned this pull request Feb 12, 2019
BethGriggs pushed a commit that referenced this pull request Feb 20, 2019
gcc 8+ recognizes that space has not been left for the pid and that the
return value of snprintf() isn't checked. Leave a little space for the
pid to prevent `-Wformat-truncation`.

PR-URL: #24810
Reviewed-By: Richard Lau <[email protected]>
BethGriggs pushed a commit that referenced this pull request Feb 20, 2019
In recent gcc, -Wrestrict warns when an argument passed to a
restrict-qualified parameter aliases with another argument.

PR-URL: #24810
Reviewed-By: Richard Lau <[email protected]>
rvagg pushed a commit that referenced this pull request Feb 28, 2019
gcc 8+ recognizes that space has not been left for the pid and that the
return value of snprintf() isn't checked. Leave a little space for the
pid to prevent `-Wformat-truncation`.

PR-URL: #24810
Reviewed-By: Richard Lau <[email protected]>
rvagg pushed a commit that referenced this pull request Feb 28, 2019
In recent gcc, -Wrestrict warns when an argument passed to a
restrict-qualified parameter aliases with another argument.

PR-URL: #24810
Reviewed-By: Richard Lau <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. inspector Issues and PRs related to the V8 inspector protocol

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants