| Message ID | 202303301017538877552@chinatelecom.cn |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [ovs-dev] util: fix an issue that thread name cannot be set | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/intel-ovs-compilation | success | test: success |
On 3/30/23 04:17, Songtao Zhan wrote: > > To: dev@openvswitch.org, > i.maximets@ovn.org > > The name of the current thread consists of a name with a maximum > length of 16 bytes and a thread ID. The final name may be longer > than 16 bytes. If the name is longer than 16 bytes, the thread > name will fail to be set > > Signed-off-by: Songtao Zhan <zhanst1@chinatelecom.cn> Looks reasonable to me, thanks! See some minor comments below. Best regards, Ilya Maximets. > --- > lib/util.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/lib/util.c b/lib/util.c > index 96a71550d..023829694 100644 > --- a/lib/util.c > +++ b/lib/util.c > @@ -645,6 +645,10 @@ set_subprogram_name(const char *subprogram_name) > free(subprogram_name_set(pname)); > > #if HAVE_GLIBC_PTHREAD_SETNAME_NP > + /* The maximum thead name including '\0' supported by the system is 16 */ I'd remove 'by the system' from the sentence above. And the sentence should end with a period. > + if (strlen(pname) > 15) { > + pname[15] = '\0' ; No need for the extra space before ';'. > + } > pthread_setname_np(pthread_self(), pname); > #elif HAVE_NETBSD_PTHREAD_SETNAME_NP > pthread_setname_np(pthread_self(), "%s", pname);
On 30 Mar 2023, at 4:17, Songtao Zhan wrote: > To: dev@openvswitch.org, > i.maximets@ovn.org > > The name of the current thread consists of a name with a maximum > length of 16 bytes and a thread ID. The final name may be longer > than 16 bytes. If the name is longer than 16 bytes, the thread > name will fail to be set Thanks for the patch, do you have examples of when this happens? Maybe we should also change the thread naming to avoid this? See one additional item below. > Signed-off-by: Songtao Zhan <zhanst1@chinatelecom.cn> > --- > lib/util.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/lib/util.c b/lib/util.c > index 96a71550d..023829694 100644 > --- a/lib/util.c > +++ b/lib/util.c > @@ -645,6 +645,10 @@ set_subprogram_name(const char *subprogram_name) > free(subprogram_name_set(pname)); > > #if HAVE_GLIBC_PTHREAD_SETNAME_NP > + /* The maximum thead name including '\0' supported by the system is 16 */ > + if (strlen(pname) > 15) { > + pname[15] = '\0' ; > + } Not sure what is better, but would it make sense to use the last upper characters? This way if we do truncate we know the internal thread id, as naming in OVS, in general, is “xasprintf("%s%u", aux.name, id)”. If we do this we could add some indication is was truncated, like “LONGTHREAD123456” would become “>NGTHREAD123456”. > pthread_setname_np(pthread_self(), pname); > #elif HAVE_NETBSD_PTHREAD_SETNAME_NP > pthread_setname_np(pthread_self(), "%s", pname); > -- > 2.31.1 > > > > zhanst1@chinatelecom.cn > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
diff --git a/lib/util.c b/lib/util.c index 96a71550d..023829694 100644 --- a/lib/util.c +++ b/lib/util.c @@ -645,6 +645,10 @@ set_subprogram_name(const char *subprogram_name) free(subprogram_name_set(pname)); #if HAVE_GLIBC_PTHREAD_SETNAME_NP + /* The maximum thead name including '\0' supported by the system is 16 */ + if (strlen(pname) > 15) { + pname[15] = '\0' ; + } pthread_setname_np(pthread_self(), pname); #elif HAVE_NETBSD_PTHREAD_SETNAME_NP pthread_setname_np(pthread_self(), "%s", pname);
To: dev@openvswitch.org, i.maximets@ovn.org The name of the current thread consists of a name with a maximum length of 16 bytes and a thread ID. The final name may be longer than 16 bytes. If the name is longer than 16 bytes, the thread name will fail to be set Signed-off-by: Songtao Zhan <zhanst1@chinatelecom.cn> --- lib/util.c | 4 ++++ 1 file changed, 4 insertions(+)