You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by "hnakamur (via GitHub)" <gi...@apache.org> on 2025/02/11 00:40:42 UTC
[PR] Deprecate name_get() and value_get() function returns const char * (#10153) [trafficserver]
hnakamur opened a new pull request, #12027:
URL: https://github.com/apache/trafficserver/pull/12027
Confirmed that `cmake --build build --target test` passes with the modification of [Static link opentelemetry-cpp libraries to otel_tracer plugin by hnakamur · Pull Request #12026 · apache/trafficserver](https://github.com/apache/trafficserver/pull/12026).
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Deprecate name_get() and value_get() function returns const char * (#10153) [trafficserver]
Posted by "hnakamur (via GitHub)" <gi...@apache.org>.
hnakamur commented on PR #12027:
URL: https://github.com/apache/trafficserver/pull/12027#issuecomment-2687849539
Should `name_get()` and `value_get()` in `MIMEFieldWrapper` be changed to return `string_view` too?
I made a commit for this change at https://github.com/hnakamur/trafficserver/commit/417849bfb905fbbad5a6f408c0a78dc94ae419ee
I can include it in the pull request or I'm going to create another pull request after this pull request is merged.
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Deprecate name_get() and value_get() function returns const char * (#10153) [trafficserver]
Posted by "masaori335 (via GitHub)" <gi...@apache.org>.
masaori335 commented on PR #12027:
URL: https://github.com/apache/trafficserver/pull/12027#issuecomment-2689369269
Rocky build is failed. It looks like QPACK also needs to be changed.
```
../src/proxy/http3/QPACK.cc: In member function 'int QPACK::_encode_header(const MIMEField&, uint16_t, IOBufferBlock*, uint16_t&)':
../src/proxy/http3/QPACK.cc:370:44: error: no matching function for call to 'MIMEField::name_get(int*) const'
370 | const char *name = field.name_get(&name_len);
| ~~~~~~~~~~~~~~^~~~~~~~~~~
In file included from ../include/proxy/hdrs/HTTP.h:30,
from ../src/proxy/http3/QPACK.cc:24:
../include/proxy/hdrs/MIME.h:145:20: note: candidate: 'std::string_view MIMEField::name_get() const'
145 | std::string_view name_get() const;
| ^~~~~~~~
../include/proxy/hdrs/MIME.h:145:20: note: candidate expects 0 arguments, 1 provided
../src/proxy/http3/QPACK.cc:376:38: error: no matching function for call to 'MIMEField::value_get(int*) const'
376 | const char *value = field.value_get(&value_len);
| ~~~~~~~~~~~~~~~^~~~~~~~~~~~
In file included from ../include/proxy/hdrs/HTTP.h:30,
from ../src/proxy/http3/QPACK.cc:24:
../include/proxy/hdrs/MIME.h:162:20: note: candidate: 'std::string_view MIMEField::value_get() const'
162 | std::string_view value_get() const;
| ^~~~~~~~~
../include/proxy/hdrs/MIME.h:162:20: note: candidate expects 0 arguments, 1 provided
```
For the `MIMEFieldWrapper`, let's do it on another PR. This PR is already big enough :)
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Deprecate name_get() and value_get() function returns const char * (#10153) [trafficserver]
Posted by "hnakamur (via GitHub)" <gi...@apache.org>.
hnakamur commented on PR #12027:
URL: https://github.com/apache/trafficserver/pull/12027#issuecomment-2682188366
I changed the signature of `marshalStr` in LogUtils.cc at [2d6f576](https://github.com/apache/trafficserver/pull/12027/commits/2d6f576ca032b647e7af74494d92f9a980555c71).
I removed the `const char*` version of `name_get` and `value_get` in `MIMEField` at [8b046b2](https://github.com/apache/trafficserver/pull/12027/commits/8b046b289dbf02bcda0a5d8f2606a71ea57e491a).
I confirmed the unit tests pass with the following commands:
```
rm -rf build
cmake -B build
cmake --build build -j -v
sudo cmake --install build
cmake --build build --target test
```
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Deprecate name_get() and value_get() function returns const char * (#10153) [trafficserver]
Posted by "masaori335 (via GitHub)" <gi...@apache.org>.
masaori335 commented on PR #12027:
URL: https://github.com/apache/trafficserver/pull/12027#issuecomment-2683491049
[approve ci]
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Deprecate name_get() and value_get() function returns const char * (#10153) [trafficserver]
Posted by "bneradt (via GitHub)" <gi...@apache.org>.
bneradt commented on PR #12027:
URL: https://github.com/apache/trafficserver/pull/12027#issuecomment-2679927420
Are there any other uses of this API now deprecated in this PR? If not, let's just remove the API rather than deprecate.
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Deprecate name_get() and value_get() function returns const char * (#10153) [trafficserver]
Posted by "hnakamur (via GitHub)" <gi...@apache.org>.
hnakamur commented on PR #12027:
URL: https://github.com/apache/trafficserver/pull/12027#issuecomment-2682279202
I thinks it's ready for review.
Could you take another look?
Thanks!
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Deprecate name_get() and value_get() function returns const char * (#10153) [trafficserver]
Posted by "hnakamur (via GitHub)" <gi...@apache.org>.
hnakamur commented on PR #12027:
URL: https://github.com/apache/trafficserver/pull/12027#issuecomment-2690514386
@masaori335 Thanks for showing me errors in Rocky build. I fixed them in [7225790](https://github.com/apache/trafficserver/pull/12027/commits/7225790c588c9082410581fd865030ce542e543e).
> For the MIMEFieldWrapper, let's do it on another PR. This PR is already big enough :)
Alright, I'm going to do it on another PR after this pull request is merged.
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Deprecate name_get() and value_get() function returns const char * (#10153) [trafficserver]
Posted by "hnakamur (via GitHub)" <gi...@apache.org>.
hnakamur commented on PR #12027:
URL: https://github.com/apache/trafficserver/pull/12027#issuecomment-2681282878
I found some uses of the API and switched them to string_view version in [a1bdbcb](https://github.com/apache/trafficserver/pull/12027/commits/a1bdbcbd00627f29e2b1f6dacc2c37a9006f5664).
The two uses are left at
https://github.com/apache/trafficserver/blob/a1bdbcbd00627f29e2b1f6dacc2c37a9006f5664/src/proxy/logging/LogUtils.cc#L475-L476
```
marshalStr(bw, mfp, &MIMEField::name_get);
marshalStr(bw, mfp, &MIMEField::value_get);
```
Can I change the signature of `marshalStr` from
```
void
marshalStr(swoc::FixedBufferWriter &bw, const MIMEField &mf, const char *(MIMEField::*get_func)(int *length) const)
```
to
```
void
marshalStr(swoc::FixedBufferWriter &bw, const MIMEField &mf, std::string_view (MIMEField::*get_func)() const)
```
, or should I change it in another pull request?
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Deprecate name_get() and value_get() function returns const char * (#10153) [trafficserver]
Posted by "masaori335 (via GitHub)" <gi...@apache.org>.
masaori335 merged PR #12027:
URL: https://github.com/apache/trafficserver/pull/12027
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org