Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

In TsSharedMutex.h, make error reporting thread-safe. #8636

Merged
merged 1 commit into from
Feb 2, 2022

Conversation

ywkaras
Copy link
Contributor

@ywkaras ywkaras commented Jan 31, 2022

No description provided.

@ywkaras ywkaras self-assigned this Jan 31, 2022
@ywkaras ywkaras force-pushed the cstring_TsSharedMutex branch from f9e371c to 72209a9 Compare January 31, 2022 17:28
@ywkaras ywkaras changed the title Add missing header file in TsSharedMutex.h. In TsSharedMutex.h, make error reporting thread-safe. Jan 31, 2022
@ywkaras ywkaras force-pushed the cstring_TsSharedMutex branch 2 times, most recently from 7e0c281 to a79511f Compare January 31, 2022 18:40
@apache apache deleted a comment from ezelkow1 Jan 31, 2022
@ywkaras ywkaras closed this Jan 31, 2022
@ywkaras ywkaras reopened this Jan 31, 2022
@ywkaras ywkaras marked this pull request as ready for review January 31, 2022 23:06
@ywkaras ywkaras requested a review from bryancall as a code owner January 31, 2022 23:06
@ywkaras ywkaras added this to the 10.0.0 milestone Jan 31, 2022
@bryancall
Copy link
Contributor

@serrislew is going to look at this

@ywkaras ywkaras force-pushed the cstring_TsSharedMutex branch from a79511f to cdcbc6f Compare February 1, 2022 00:40
Strerror(int err_num)
{
_c_str = strerror_r(err_num, _buf, 256);
if (!_c_str) {
Copy link
Contributor

Choose a reason for hiding this comment

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

some docs show that with the GNU-specific strerror_r(), if err_num is unknown, it can return an "unknown error nnn" message which won't make _c_str null, making the conditional think that strerror_r() did not fail when it did. Maybe use the XSI-compliant version (returning int) and returning _buf which also has the error string

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TS is compiled with -D_GNU_SOURCE, which means that only the GNU-specific strerror_r() is available. Even if it never returns null (currently), I'd rather leave the handling in for future-safety.

Copy link
Contributor

Choose a reason for hiding this comment

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

This breaks build on macOS #8642

Copy link
Contributor

@vmamidi vmamidi left a comment

Choose a reason for hiding this comment

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

Approving for Serris Lew.

@ywkaras ywkaras merged commit 4b0d3cb into apache:master Feb 2, 2022
zwoop pushed a commit that referenced this pull request Feb 17, 2022
@zwoop
Copy link
Contributor

zwoop commented Feb 17, 2022

Cherry-picked to v9.2.x

@zwoop zwoop modified the milestones: 10.0.0, 9.2.0 Feb 17, 2022
moonchen pushed a commit to moonchen/trafficserver that referenced this pull request Mar 17, 2022
* asf/9.2.x:
  Revert "DNS: Fix lack of nameserver failover in low use circumstances. (apache#7843)" (apache#8663)
  Fix strategies to initialize scheme (apache#8650)
  DNS: Fix lack of nameserver failover in low use circumstances. (apache#7843)
  Cleanup strategy debug logs (apache#8656)
  Making 9.2.x backwards compatible with 9.1.x (apache#8661)
  Adds two overridable config variables to control parent mark downs. (apache#8595)
  Fix plugin parent_select missing hostname len (apache#8649)
  Ports apache#7925 apache#8365 core to parent_select plugin (apache#8590)
  Ports apache#7897 from core strategies to parent_select plugin. (apache#8580)
  Adding clangd language server files to .gitignore (apache#8640)
  Make TsSharedMutex.h compile on MacOS. (apache#8645)
  In TsSharedMutex.h, make error reporting thread-safe. (apache#8636)
  Revert "body factory does not respect runroot (apache#8388)" (apache#8654)
  doc: Convert miscased Traffic Server references to |TS| macro (apache#8543)
  Add a new --enable-event-tracker configure option (apache#8179)
  Add parent_select plugin strategy caching (apache#8651)
  TLS Session Resumption: fix timed out session (apache#8667)
  Fix to allow running  from outside top_srcdir (apache#8673)
  Send diags output to stderr when running regression tests. (apache#8662)
  Default proxy.config.http.strict_uri_parsing to "2" (apache#8632)
ywkaras pushed a commit to ywkaras/trafficserver that referenced this pull request Jul 7, 2022
ywkaras pushed a commit to ywkaras/trafficserver that referenced this pull request Jul 7, 2022
In TsSharedMutex.h, make error reporting thread-safe. (apache#8636)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants