You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brpc.apache.org by "ZhengweiZhu (via GitHub)" <gi...@apache.org> on 2025/11/20 06:43:57 UTC
[PR] Support naming bthread to help debug (brpc)
ZhengweiZhu opened a new pull request, #3158:
URL: https://github.com/apache/brpc/pull/3158
The bthread name is shown when checking bthread status by curl ip:port/bthreads/xxx, which helps to debug when bthread trace is not enabled.
### What problem does this PR solve?
Issue Number: #3088
Problem Summary:
### What is changed and the side effects?
Changed:
add `name` field in bthread_attr_t and set name when creating bthread
Side effects:
- Performance effects: NO
- Breaking backward compatibility: NO
---
### Check List:
- Please make sure your changes are compilable.
- When providing us with a new feature, it is best to add related tests.
- Please follow [Contributor Covenant Code of Conduct](https://github.com/apache/brpc/blob/master/CODE_OF_CONDUCT.md).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
Re: [PR] Support naming bthread to help debug (brpc)
Posted by "wwbmmm (via GitHub)" <gi...@apache.org>.
wwbmmm commented on code in PR #3158:
URL: https://github.com/apache/brpc/pull/3158#discussion_r2549639039
##########
src/bthread/types.h:
##########
@@ -118,6 +120,12 @@ typedef struct bthread_attr_t {
return tmp;
}
#endif // __cplusplus
+ void set_name(const char* val) {
+ if (val) {
+ strncpy(name, val, BTHREAD_NAME_MAX_LENGTH);
+ name[BTHREAD_NAME_MAX_LENGTH] = '\0';
Review Comment:
Actually there is a `bthread_attr_init` function. It is the right way to init a `bthread_attr_t` without using pre-defined macros like 'BTHREAD_ATTR_*'. This is a more extensible way.
In current way, some users may define `BTHREAD_ATTR_XXX` by themselves, but that code will be broken if we add new members to `bthread_attr_t`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
Re: [PR] Support naming bthread to help debug (brpc)
Posted by "ZhengweiZhu (via GitHub)" <gi...@apache.org>.
ZhengweiZhu commented on code in PR #3158:
URL: https://github.com/apache/brpc/pull/3158#discussion_r2548279327
##########
src/bthread/types.h:
##########
@@ -97,12 +97,14 @@ typedef struct {
size_t nfree;
} bthread_keytable_pool_stat_t;
+static const size_t BTHREAD_NAME_MAX_LENGTH = 31;
// Attributes for thread creation.
typedef struct bthread_attr_t {
bthread_stacktype_t stack_type;
bthread_attrflags_t flags;
bthread_keytable_pool_t* keytable_pool;
bthread_tag_t tag;
+ char name[BTHREAD_NAME_MAX_LENGTH + 1]; // do not use std::string to keep POD
Review Comment:
For static constants like BTHREAD_ATTR_NORMAL, the name field is all initialized with {0}, which sets all elements of the array (including the last one) to zero. so user is not required to set name and the name will be printed as empty string, which is backward compatibility
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
Re: [PR] Support naming bthread to help debug (brpc)
Posted by "ZhengweiZhu (via GitHub)" <gi...@apache.org>.
ZhengweiZhu commented on PR #3158:
URL: https://github.com/apache/brpc/pull/3158#issuecomment-3599797898
ping
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
Re: [PR] Support naming bthread to help debug (brpc)
Posted by "ZhengweiZhu (via GitHub)" <gi...@apache.org>.
ZhengweiZhu commented on code in PR #3158:
URL: https://github.com/apache/brpc/pull/3158#discussion_r2548334165
##########
src/bthread/task_group.cpp:
##########
@@ -1141,6 +1141,7 @@ void print_task(std::ostream& os, bthread_t tid, bool enable_trace,
<< "\nattr={stack_type=" << attr.stack_type
<< " flags=" << attr.flags
<< " specified_tag=" << attr.tag
+ << " name=" << attr.name
Review Comment:
what do u mean by invalid? The empty string?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
Re: [PR] Support naming bthread to help debug (brpc)
Posted by "ZhengweiZhu (via GitHub)" <gi...@apache.org>.
ZhengweiZhu commented on code in PR #3158:
URL: https://github.com/apache/brpc/pull/3158#discussion_r2548439115
##########
src/bthread/types.h:
##########
@@ -118,6 +120,12 @@ typedef struct bthread_attr_t {
return tmp;
}
#endif // __cplusplus
+ void set_name(const char* val) {
+ if (val) {
+ strncpy(name, val, BTHREAD_NAME_MAX_LENGTH);
+ name[BTHREAD_NAME_MAX_LENGTH] = '\0';
Review Comment:
For static constants like BTHREAD_ATTR_NORMAL, the name field is initialized with {0}, which sets all elements of the array (including the last one) to zero. This means the name is already fully null-initialized, and no further action is needed to ensure it is null-terminated. This line name[BTHREAD_NAME_MAX_LENGTH] = '\0'; is actually not needed but kept here for program style.
The current implementation handles well whether the actual length being copied exceeds BTHREAD_NAME_MAX_LENGTH or not.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
Re: [PR] Support naming bthread to help debug (brpc)
Posted by "ZhengweiZhu (via GitHub)" <gi...@apache.org>.
ZhengweiZhu commented on code in PR #3158:
URL: https://github.com/apache/brpc/pull/3158#discussion_r2548433924
##########
src/bthread/types.h:
##########
@@ -118,6 +120,12 @@ typedef struct bthread_attr_t {
return tmp;
}
#endif // __cplusplus
+ void set_name(const char* val) {
Review Comment:
updated
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
Re: [PR] Support naming bthread to help debug (brpc)
Posted by "wwbmmm (via GitHub)" <gi...@apache.org>.
wwbmmm commented on code in PR #3158:
URL: https://github.com/apache/brpc/pull/3158#discussion_r2545872066
##########
src/bthread/types.h:
##########
@@ -97,12 +97,14 @@ typedef struct {
size_t nfree;
} bthread_keytable_pool_stat_t;
+static const size_t BTHREAD_NAME_MAX_LENGTH = 31;
// Attributes for thread creation.
typedef struct bthread_attr_t {
bthread_stacktype_t stack_type;
bthread_attrflags_t flags;
bthread_keytable_pool_t* keytable_pool;
bthread_tag_t tag;
+ char name[BTHREAD_NAME_MAX_LENGTH + 1]; // do not use std::string to keep POD
Review Comment:
Some existing user code may create bthread_attr_t without initialize the `name`.
For backward compatibility, I think you can add a flag to the `bthread_attrflags_t flags` like `BTHREAD_HAS_NAME`. Only when this flag is on, the name will be used.
##########
src/bthread/types.h:
##########
@@ -118,6 +120,12 @@ typedef struct bthread_attr_t {
return tmp;
}
#endif // __cplusplus
+ void set_name(const char* val) {
Review Comment:
This is C++ style interface
but the original bthread interface is designed as C style.
maybe you can add a global function `bthread_attr_set_name`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
Re: [PR] Support naming bthread to help debug (brpc)
Posted by "ZhengweiZhu (via GitHub)" <gi...@apache.org>.
ZhengweiZhu commented on code in PR #3158:
URL: https://github.com/apache/brpc/pull/3158#discussion_r2548279327
##########
src/bthread/types.h:
##########
@@ -97,12 +97,14 @@ typedef struct {
size_t nfree;
} bthread_keytable_pool_stat_t;
+static const size_t BTHREAD_NAME_MAX_LENGTH = 31;
// Attributes for thread creation.
typedef struct bthread_attr_t {
bthread_stacktype_t stack_type;
bthread_attrflags_t flags;
bthread_keytable_pool_t* keytable_pool;
bthread_tag_t tag;
+ char name[BTHREAD_NAME_MAX_LENGTH + 1]; // do not use std::string to keep POD
Review Comment:
I have add default '0' value in the const defined bthread_attr_t like BTHREAD_ATTR_NORMAL, so user is not required to set name and the name will be '0' (NULL) array, which is backward compatibility
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
Re: [PR] Support naming bthread to help debug (brpc)
Posted by "ZhengweiZhu (via GitHub)" <gi...@apache.org>.
ZhengweiZhu commented on code in PR #3158:
URL: https://github.com/apache/brpc/pull/3158#discussion_r2548905760
##########
src/bthread/types.h:
##########
@@ -118,6 +120,12 @@ typedef struct bthread_attr_t {
return tmp;
}
#endif // __cplusplus
+ void set_name(const char* val) {
+ if (val) {
+ strncpy(name, val, BTHREAD_NAME_MAX_LENGTH);
+ name[BTHREAD_NAME_MAX_LENGTH] = '\0';
Review Comment:
1. Where bthread_attr_t is used, where the predefined static constants like 'BTHREAD_ATTR_*' is used. I have not seen any code constructing bthread_attr_t manually and no reason to do so.
2. Even if 'BTHREAD_ATTR_*' is not used, current implementation also handles well. when the bthread name is set by user, the string be copied should be null terminated. if the string exceeds BTHREAD_NAME_MAX_LENGTH, then name[BTHREAD_NAME_MAX_LENGTH] = '\0'; works to avoid buffer overflow.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
Re: [PR] Support naming bthread to help debug (brpc)
Posted by "chenBright (via GitHub)" <gi...@apache.org>.
chenBright commented on code in PR #3158:
URL: https://github.com/apache/brpc/pull/3158#discussion_r2546493464
##########
src/bthread/types.h:
##########
@@ -118,6 +120,12 @@ typedef struct bthread_attr_t {
return tmp;
}
#endif // __cplusplus
+ void set_name(const char* val) {
+ if (val) {
+ strncpy(name, val, BTHREAD_NAME_MAX_LENGTH);
+ name[BTHREAD_NAME_MAX_LENGTH] = '\0';
Review Comment:
The null terminator ('\0') should be set according to the actual length of the string being copied.
##########
src/bthread/task_group.cpp:
##########
@@ -1141,6 +1141,7 @@ void print_task(std::ostream& os, bthread_t tid, bool enable_trace,
<< "\nattr={stack_type=" << attr.stack_type
<< " flags=" << attr.flags
<< " specified_tag=" << attr.tag
+ << " name=" << attr.name
Review Comment:
The name may be invalid.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
Re: [PR] Support naming bthread to help debug (brpc)
Posted by "wwbmmm (via GitHub)" <gi...@apache.org>.
wwbmmm commented on code in PR #3158:
URL: https://github.com/apache/brpc/pull/3158#discussion_r2554512358
##########
src/bthread/types.h:
##########
@@ -118,6 +120,12 @@ typedef struct bthread_attr_t {
return tmp;
}
#endif // __cplusplus
+ void set_name(const char* val) {
+ if (val) {
+ strncpy(name, val, BTHREAD_NAME_MAX_LENGTH);
+ name[BTHREAD_NAME_MAX_LENGTH] = '\0';
Review Comment:
>
> Yes. As the bthread_attr_t struct is defined in C code style without defining ctor function, so when new members is added and user have defined its BTHREAD_ATTR_XXX by themselves and not used `bthread_attr_init`, the new added name filed will be uninitialized. Shall we consider this scenario and how?
I think the current code is OK. Maybe we need to add to document that user must use `bthread_attr_init` when they initialize `bthread_attr_t` without using pre-defined macros.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
Re: [PR] Support naming bthread to help debug (brpc)
Posted by "Regal2XY (via GitHub)" <gi...@apache.org>.
Regal2XY commented on code in PR #3158:
URL: https://github.com/apache/brpc/pull/3158#discussion_r2548364143
##########
src/bthread/types.h:
##########
@@ -118,6 +120,12 @@ typedef struct bthread_attr_t {
return tmp;
}
#endif // __cplusplus
+ void set_name(const char* val) {
+ if (val) {
+ strncpy(name, val, BTHREAD_NAME_MAX_LENGTH);
+ name[BTHREAD_NAME_MAX_LENGTH] = '\0';
Review Comment:
For static constants like BTHREAD_ATTR_NORMAL, the name field is initialized with {0}, which sets all elements of the array (including the last one) to zero. This means the name is already fully null-initialized, and no further action is needed to ensure it is null-terminated as we only strncpy BTHREAD_NAME_MAX_LENGTH (which is less than array size) elements. This line is actually not needed but kept here just for coding style. Should I remove this line?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
Re: [PR] Support naming bthread to help debug (brpc)
Posted by "ZhengweiZhu (via GitHub)" <gi...@apache.org>.
ZhengweiZhu commented on code in PR #3158:
URL: https://github.com/apache/brpc/pull/3158#discussion_r2553838219
##########
src/bthread/types.h:
##########
@@ -118,6 +120,12 @@ typedef struct bthread_attr_t {
return tmp;
}
#endif // __cplusplus
+ void set_name(const char* val) {
+ if (val) {
+ strncpy(name, val, BTHREAD_NAME_MAX_LENGTH);
+ name[BTHREAD_NAME_MAX_LENGTH] = '\0';
Review Comment:
> Actually there is a `bthread_attr_init` function. It is the right way to init a `bthread_attr_t` without using pre-defined macros like 'BTHREAD_ATTR_*'. This is a more extensible way.
> In current way, some users may define `BTHREAD_ATTR_XXX` by themselves, but that code will be broken if we add new members to `bthread_attr_t`.
Yes. As the bthread_attr_t struct is defined in C code style without defining ctor function, so when new members is added and user have defined its BTHREAD_ATTR_XXX by themselves and not used `bthread_attr_init`, the new added name filed will be uninitialized. Shall we consider this scenario and how?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
Re: [PR] Support naming bthread to help debug (brpc)
Posted by "chenBright (via GitHub)" <gi...@apache.org>.
chenBright commented on code in PR #3158:
URL: https://github.com/apache/brpc/pull/3158#discussion_r2549277696
##########
src/bthread/types.h:
##########
@@ -118,6 +120,12 @@ typedef struct bthread_attr_t {
return tmp;
}
#endif // __cplusplus
+ void set_name(const char* val) {
+ if (val) {
+ strncpy(name, val, BTHREAD_NAME_MAX_LENGTH);
+ name[BTHREAD_NAME_MAX_LENGTH] = '\0';
Review Comment:
OK. My concern is that If 'BTHREAD_ATTR_*' is not used and the bthread name is not set, the bthread name is random char. Other fields also have this issue, but so far they're all working fine.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
Re: [PR] Support naming bthread to help debug (brpc)
Posted by "Regal2XY (via GitHub)" <gi...@apache.org>.
Regal2XY commented on code in PR #3158:
URL: https://github.com/apache/brpc/pull/3158#discussion_r2548364143
##########
src/bthread/types.h:
##########
@@ -118,6 +120,12 @@ typedef struct bthread_attr_t {
return tmp;
}
#endif // __cplusplus
+ void set_name(const char* val) {
+ if (val) {
+ strncpy(name, val, BTHREAD_NAME_MAX_LENGTH);
+ name[BTHREAD_NAME_MAX_LENGTH] = '\0';
Review Comment:
For static constants like BTHREAD_ATTR_NORMAL, the name field is initialized with {0}, which sets all elements of the array (including the last one) to zero. This means the name is already fully null-initialized, and no further action is needed to ensure it is null-terminated as we only strncpy BTHREAD_NAME_MAX_LENGTH (which is less than array size) elements. This line is actually not needed but kept here just for coding style.
The current implementation handles well whether the length of name execeeds BTHREAD_NAME_MAX_LENGTH or not.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
Re: [PR] Support naming bthread to help debug (brpc)
Posted by "chenBright (via GitHub)" <gi...@apache.org>.
chenBright merged PR #3158:
URL: https://github.com/apache/brpc/pull/3158
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
Re: [PR] Support naming bthread to help debug (brpc)
Posted by "ZhengweiZhu (via GitHub)" <gi...@apache.org>.
ZhengweiZhu commented on code in PR #3158:
URL: https://github.com/apache/brpc/pull/3158#discussion_r2555264587
##########
src/bthread/types.h:
##########
@@ -118,6 +120,12 @@ typedef struct bthread_attr_t {
return tmp;
}
#endif // __cplusplus
+ void set_name(const char* val) {
+ if (val) {
+ strncpy(name, val, BTHREAD_NAME_MAX_LENGTH);
+ name[BTHREAD_NAME_MAX_LENGTH] = '\0';
Review Comment:
I think the advanced usage of user defined BTHREAD_ATTR_XXX is really rare and if so they should be able to handle the compatibility. I have not found any document suitable for explaining it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
Re: [PR] Support naming bthread to help debug (brpc)
Posted by "Regal2XY (via GitHub)" <gi...@apache.org>.
Regal2XY commented on code in PR #3158:
URL: https://github.com/apache/brpc/pull/3158#discussion_r2548364143
##########
src/bthread/types.h:
##########
@@ -118,6 +120,12 @@ typedef struct bthread_attr_t {
return tmp;
}
#endif // __cplusplus
+ void set_name(const char* val) {
+ if (val) {
+ strncpy(name, val, BTHREAD_NAME_MAX_LENGTH);
+ name[BTHREAD_NAME_MAX_LENGTH] = '\0';
Review Comment:
For static constants like BTHREAD_ATTR_NORMAL, the name field is initialized with {0}, which sets all elements of the array (including the last one) to zero. This means the name is already fully null-initialized, and no further action is needed to ensure it is null-terminated as we only strncpy BTHREAD_NAME_MAX_LENGTH (which is less than array size) elements. This line is actually not needed but kept here just for coding style.
The current implementation handles well whether the length of name execeeds BTHREAD_NAME_MAX_LENGTH or not.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
Re: [PR] Support naming bthread to help debug (brpc)
Posted by "chenBright (via GitHub)" <gi...@apache.org>.
chenBright commented on code in PR #3158:
URL: https://github.com/apache/brpc/pull/3158#discussion_r2548561334
##########
src/bthread/types.h:
##########
@@ -118,6 +120,12 @@ typedef struct bthread_attr_t {
return tmp;
}
#endif // __cplusplus
+ void set_name(const char* val) {
+ if (val) {
+ strncpy(name, val, BTHREAD_NAME_MAX_LENGTH);
+ name[BTHREAD_NAME_MAX_LENGTH] = '\0';
Review Comment:
What if BTHREAD_ATTR_NORMAL is not used?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
Re: [PR] Support naming bthread to help debug (brpc)
Posted by "wwbmmm (via GitHub)" <gi...@apache.org>.
wwbmmm commented on code in PR #3158:
URL: https://github.com/apache/brpc/pull/3158#discussion_r2548449560
##########
src/bthread/types.h:
##########
@@ -97,12 +97,14 @@ typedef struct {
size_t nfree;
} bthread_keytable_pool_stat_t;
+static const size_t BTHREAD_NAME_MAX_LENGTH = 31;
// Attributes for thread creation.
typedef struct bthread_attr_t {
bthread_stacktype_t stack_type;
bthread_attrflags_t flags;
bthread_keytable_pool_t* keytable_pool;
bthread_tag_t tag;
+ char name[BTHREAD_NAME_MAX_LENGTH + 1]; // do not use std::string to keep POD
Review Comment:
OK, I see the `bthread_attr_init` function use `BTHREAD_ATTR_NORMAL`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
Re: [PR] Support naming bthread to help debug (brpc)
Posted by "ZhengweiZhu (via GitHub)" <gi...@apache.org>.
ZhengweiZhu commented on code in PR #3158:
URL: https://github.com/apache/brpc/pull/3158#discussion_r2548460431
##########
src/bthread/types.h:
##########
@@ -97,12 +97,14 @@ typedef struct {
size_t nfree;
} bthread_keytable_pool_stat_t;
+static const size_t BTHREAD_NAME_MAX_LENGTH = 31;
// Attributes for thread creation.
typedef struct bthread_attr_t {
bthread_stacktype_t stack_type;
bthread_attrflags_t flags;
bthread_keytable_pool_t* keytable_pool;
bthread_tag_t tag;
+ char name[BTHREAD_NAME_MAX_LENGTH + 1]; // do not use std::string to keep POD
Review Comment:
actually there's no usage of bthread_attr_init. BTHREAD_ATTR_NORMAL is used when attr is null. 😂
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
Re: [PR] Support naming bthread to help debug (brpc)
Posted by "wwbmmm (via GitHub)" <gi...@apache.org>.
wwbmmm commented on PR #3158:
URL: https://github.com/apache/brpc/pull/3158#issuecomment-3599866009
LGTM
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org