trafficserver icon indicating copy to clipboard operation
trafficserver copied to clipboard

Deprecate name_get() and value_get() function returns `const char *`

Open masaori335 opened this issue 1 year ago • 11 comments

Proposal

Deprecate below two functions of the MIMEField

const char *name_get(int *length) const;
const char *value_get(int *length) const;

Background

MIMEField has two value_get functions return std::string_view and const char *.

https://github.com/apache/trafficserver/blob/f9aa1e0adb7d23cb9d9dd4fb421b80a24654ae85/proxy/hdrs/MIME.h#L162-L164

Discussion on #10144, std::string_view value_get() const; is preferred for error handling and I completely agree with using std::string_view.

We can say the same things for name_get too. https://github.com/apache/trafficserver/blob/f9aa1e0adb7d23cb9d9dd4fb421b80a24654ae85/proxy/hdrs/MIME.h#L144-L146

Concerns

These functions are used widely. The change will be big.

masaori335 avatar Aug 07 '23 03:08 masaori335

Note from weekly bug scrub:


@bryancall raised a concern about performance impact. Because he saw a performance bottleneck with std::string_view version before.

However, the current implementation of the const char * version is calling the std::string_view version internally, as @SolidWallOfCode pointed out. https://github.com/apache/trafficserver/blob/f9e931441e73eb1927ab3d7b60a5c77f762cdbb8/proxy/hdrs/MIME.h#L991-L997

masaori335 avatar Aug 07 '23 23:08 masaori335

@masaori335 Can you please assign this to me,I would like to work on this.

SunMoon97 avatar Aug 18 '23 18:08 SunMoon97

@masaori335 is the above issue resolved? If not, I would like to contribute to it.

ShaiviAgarwal2 avatar Nov 14 '23 05:11 ShaiviAgarwal2

This is not resolved yet. Thanks for showing interests!

@SunMoon97 I'm really sorry, I just noticed your comment. Still want to work on this? If so, please go a head.

@ShaiviAgarwal2 if @SunMoon97 doesn't want to work on this or no reply for a while, please work on this. I'd also say we have more Good First Issues :) Good First Issue

masaori335 avatar Nov 14 '23 06:11 masaori335

Okk

ShaiviAgarwal2 avatar Nov 14 '23 09:11 ShaiviAgarwal2

Hello. can someone pls explain where can i find the definition of functions std::string_view name_get() const; and std::string_view value_get() const; in the code. I couldn't find the code for them.

Manohar-Penta avatar Dec 03 '23 19:12 Manohar-Penta

Hello. can someone pls explain where can i find the definition of functions std::string_view name_get() const; and std::string_view value_get() const; in the code. I couldn't find the code for them.

The functions std::string_view name_get() const; and std::string_view value_get() const; are member functions of the MIMEField class. The definitions of these functions would be in the corresponding source file (MIME.cc), which is typically located in the same directory as the header file (MIME.h). According to me, name_get() returns the name of the field, and value_get() returns the value of the field. Both functions include parameters to return the length of the name or value as an integer. These functions are defined in the header file for the MIMEField class

Deprecating the functions means marking them as outdated or no longer recommended for use. It is a way to inform users that these functions may be removed in future versions and should be replaced with the preferred alternatives.

ShaiviAgarwal2 avatar Dec 04 '23 18:12 ShaiviAgarwal2

@Manohar-Penta I hope I was able to solve your doubt!! If not, please acknowledge the message

ShaiviAgarwal2 avatar Dec 04 '23 18:12 ShaiviAgarwal2

Thanks Shaivi. I was able to find the definitions of the functions. and Are you still working on the issue?? If not can I take over?

Manohar-Penta avatar Dec 05 '23 09:12 Manohar-Penta

Hi, can I tackle this issue?

dmefs avatar May 06 '24 17:05 dmefs

Should I change char * version to std::string_view in all places or adding a warning is enough?

dmefs avatar May 06 '24 18:05 dmefs