Skip to content

src: check empty before accessing string - #51665

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
zcbenz:fix-str-front-assert
Feb 16, 2024
Merged

src: check empty before accessing string#51665
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
zcbenz:fix-str-front-assert

Conversation

@zcbenz

Copy link
Copy Markdown
Contributor

Fix an assertion when running dotnev tests with GN build:
assertion !empty() failed: string::front(): string is empty

which was caused by calling value.front() without verifying the value is not empty.

@nodejs-github-botnodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. labels Feb 5, 2024
@zcbenz
zcbenzforce-pushed the fix-str-front-assert branch from 22199ff to 7da4605CompareFebruary 5, 2024 01:32
@zcbenz
zcbenz marked this pull request as draft February 5, 2024 01:39
Fix an assertion when running dotnev tests with GN build:
assertion !empty() failed: string::front(): string is empty
which was caused by calling value.front() without verifying the value is
not empty.
@zcbenz
zcbenzforce-pushed the fix-str-front-assert branch from 7da4605 to b208e65CompareFebruary 5, 2024 01:46
@zcbenz
zcbenz marked this pull request as ready for review February 5, 2024 01:48

@anonriganonrig left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice catch

@anonriganonrig added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 5, 2024
@zcbenz

Copy link
Copy Markdown
ContributorAuthor

Can this be merged? It is currently breaking GN build.

@FlarnaFlarna added request-ci Add this label to start a Jenkins CI on a PR. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Feb 13, 2024
@marco-ippolito

Copy link
Copy Markdown
Member

@zcbenz CI is currently locked for the security release

@victorgomes

Copy link
Copy Markdown

Another duplicate: #51757

@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 14, 2024
@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@FlarnaFlarna added the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 16, 2024
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 16, 2024
@nodejs-github-bot
nodejs-github-bot merged commit c682fbf into nodejs:mainFeb 16, 2024
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Landed in c682fbf

targos pushed a commit that referenced this pull request Feb 19, 2024
Fix an assertion when running dotnev tests with GN build:
assertion !empty() failed: string::front(): string is empty
which was caused by calling value.front() without verifying the value is
not empty.
PR-URL: #51665
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
@marco-ippolitomarco-ippolito mentioned this pull request Mar 1, 2024
richardlau pushed a commit that referenced this pull request Mar 25, 2024
Fix an assertion when running dotnev tests with GN build:
assertion !empty() failed: string::front(): string is empty
which was caused by calling value.front() without verifying the value is
not empty.
PR-URL: #51665
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
@richardlaurichardlau mentioned this pull request Mar 25, 2024
Sign up for freeto 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++.needs-ciPRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants

@zcbenz@marco-ippolito@victorgomes@nodejs-github-bot@lpinca@anonrig@Flarna