You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@trafficserver.apache.org by Walt Karas <wk...@yahooinc.com.INVALID> on 2022/09/01 01:08:03 UTC
Re: [E] Re: Proposed change to TS API: TSHeapBuf
As a rule of thumb, I prefer using encapsulation/data abstraction. I think
perhaps that is one reason I've been a poor match to this project. There
doesn't seem to be a consensus that we should follow this rule of thumb.
KIt, that would be my preference. But I am part of the consensus I think
we have, that we should favor a series of smaller steps, rather than doing
all of them in one big step.
On Wed, Aug 31, 2022 at 12:13 PM Shu Kit Chan <ch...@gmail.com> wrote:
> Also are we planning to eventually rewrite our existing APIs (where
> applicable) to use this?
>
> On Wed, Aug 31, 2022 at 8:36 AM Masakazu Kitajo <ma...@apache.org> wrote:
> >
> > What's the advantage of using TSHeapBuf? What issue does it solve?
> >
> >
> > On Wed, Aug 31, 2022 at 7:48 AM Walt Karas <wk...@yahooinc.com.invalid>
> > wrote:
> >
> > > Described here:
> > >
> > >
> https://urldefense.com/v3/__https://github.com/apache/trafficserver/blob/os_pkey_cnf_reload/doc/developer-guide/api/functions/TSHeapBuf.en.rst*tsheapbufdata__;Iw!!Op6eflyXZCqGR5I!Du0fBfMhb4pdM2ECFijJ7aJ-jT70jEPeZwjhsvWt2Dr2cSZ5G7HWY20wZOmFHIR3MxnvPZpoRDMlII5dgow$
>
> > > ,
> > >
> > > In PR
> https://urldefense.com/v3/__https://github.com/apache/trafficserver/pull/8790__;!!Op6eflyXZCqGR5I!Du0fBfMhb4pdM2ECFijJ7aJ-jT70jEPeZwjhsvWt2Dr2cSZ5G7HWY20wZOmFHIR3MxnvPZpoRDMlYh9lIuc$
> .
> > >
> > > This allows a dynamically allocated buffer, of any reasonable length,
> to be
> > > returned by a TS API function, like this:
> > >
> > > TSHeapBuf hb = TSSomething(x, y, z);
> > >
> > > One alternative is an interface like this:
> > >
> > > int length;
> > > char *data = TSSomething(x, y, z, &length);
> > >
> > > The data is dynamically allocated, and would be freed with TSfree().
> > >
> > > Another alternative is:
> > >
> > > char *buf = TSalloc(BUF_SIZE);
> > > int actual_size = TSSomething(x, y, z, buf, BUF_SIZE);
> > > if (actual_size > BUF_SIZE) {
> > > // buf was too small, unchanged.
> > > TSfree(buf);
> > > buf = TSalloc(actual_size);
> > > TSSomething(x, y, z, buf, actual_size);
> > > }
> > >
>
Re: [E] Proposed change to TS API: TSHeapBuf
Posted by Walt Karas <wk...@yahooinc.com.INVALID>.
Oh yeah I forgot, isn't a big reason to keep the TS API as C that it allows
you to write programs in Rust? And can proxy-wasm handle it OK if the TS
API is in C++? How about callbacks from Lua scripts being run by the Lua
plugin?
On Wed, Jan 4, 2023 at 4:26 PM Leif Hedstrom <zw...@apache.org> wrote:
>
>
> > On Jan 4, 2023, at 3:03 PM, Walt Karas <wk...@yahooinc.com.INVALID>
> wrote:
> >
> > There are people at Yahoo who have written plugins in straight C, and
> don't
> > want to change to C++.
>
> They can continue to write plugins in C, just have to compile with a C++
> compiler. I imagine there will be very few, if any changes. Of course, they
> can’t use any new APIs, or new structures added that are C++.
>
> — leif
> >
> > On Wed, Jan 4, 2023 at 1:10 PM Leif Hedstrom <zw...@apache.org> wrote:
> >
> >> This looks like a string_view to me?
> >>
> >> Why don’t we do what we talked about for years now, and stop supporting
> >> compiling plug-ins with C but require C++? Then we can introduce nice
> >> things in ts.h. The old C plugins just have to be changed to compile
> with
> >> C++, and ABI compatibility within major version must be enforced.
> >>
> >> — Leif
> >>
> >>> On Sep 6, 2022, at 10:24, Walt Karas <wk...@yahooinc.com.invalid>
> >> wrote:
> >>>
> >>> Presumably we don't need to allow for use of antiquated C compilers
> that
> >>> don't allow structures as return values. So this:
> >>>
> >>> typedef struct
> >>> {
> >>> char *data;
> >>> int length;
> >>> }
> >>> TSVarLenData;
> >>>
> >>> TSVarLenData TSSomething(x, y, z);
> >>>
> >>> Is another alternative.
> >>>
> >>>> On Thu, Sep 1, 2022 at 5:32 PM Masakazu Kitajo <ma...@apache.org>
> >> wrote:
> >>>>
> >>>> Using encapsulation/data abstraction is fine, but it doesn't seem like
> >>>> TSHeapBuf makes sense. No TS API receives it. If plugin code is the
> only
> >>>> user and it has to deal with a non-const raw pointer and a data length
> >>>> after all, why do we want to wrap them in the first place?
> >>>>
> >>>> As for smaller steps, like I suggested on the PR, you could introduce
> >>>> TSHeapBuf separately from the fix, I think. And if there are similar
> TS
> >>>> APIs that could return TSHeapBuf, supporting TSHeapBuf on only one of
> >> them
> >>>> makes inconsistency in TS API. IMO, the two changes, the fix and the
> new
> >>>> API, should be made like 1 + 1, but not 1.5 + 0.5 nor 1 + 0.5 + 0.5.
> >>>>
> >>>> Masakazu
> >>>>
> >>>> On Thu, Sep 1, 2022 at 10:08 AM Walt Karas
> <wkaras@yahooinc.com.invalid
> >>>
> >>>> wrote:
> >>>>
> >>>>> As a rule of thumb, I prefer using encapsulation/data abstraction. I
> >>>> think
> >>>>> perhaps that is one reason I've been a poor match to this project.
> >> There
> >>>>> doesn't seem to be a consensus that we should follow this rule of
> >> thumb.
> >>>>>
> >>>>> KIt, that would be my preference. But I am part of the consensus I
> >> think
> >>>>> we have, that we should favor a series of smaller steps, rather than
> >>>> doing
> >>>>> all of them in one big step.
> >>>>>
> >>>>> On Wed, Aug 31, 2022 at 12:13 PM Shu Kit Chan <ch...@gmail.com>
> >>>>> wrote:
> >>>>>
> >>>>>> Also are we planning to eventually rewrite our existing APIs (where
> >>>>>> applicable) to use this?
> >>>>>>
> >>>>>> On Wed, Aug 31, 2022 at 8:36 AM Masakazu Kitajo <ma...@apache.org>
> >>>>> wrote:
> >>>>>>>
> >>>>>>> What's the advantage of using TSHeapBuf? What issue does it solve?
> >>>>>>>
> >>>>>>>
> >>>>>>> On Wed, Aug 31, 2022 at 7:48 AM Walt Karas
> >>>> <wkaras@yahooinc.com.invalid
> >>>>>>
> >>>>>>> wrote:
> >>>>>>>
> >>>>>>>> Described here:
> >>>>>>>>
> >>>>>>>>
> >>>>>>
> >>>>>
> >>>>
> >>
> https://urldefense.com/v3/__https://github.com/apache/trafficserver/blob/os_pkey_cnf_reload/doc/developer-guide/api/functions/TSHeapBuf.en.rst*tsheapbufdata__;Iw!!Op6eflyXZCqGR5I!Du0fBfMhb4pdM2ECFijJ7aJ-jT70jEPeZwjhsvWt2Dr2cSZ5G7HWY20wZOmFHIR3MxnvPZpoRDMlII5dgow$
> >>>>>>
> >>>>>>>> ,
> >>>>>>>>
> >>>>>>>> In PR
> >>>>>>
> >>>>>
> >>>>
> >>
> https://urldefense.com/v3/__https://github.com/apache/trafficserver/pull/8790__;!!Op6eflyXZCqGR5I!Du0fBfMhb4pdM2ECFijJ7aJ-jT70jEPeZwjhsvWt2Dr2cSZ5G7HWY20wZOmFHIR3MxnvPZpoRDMlYh9lIuc$
> >>>>>> .
> >>>>>>>>
> >>>>>>>> This allows a dynamically allocated buffer, of any reasonable
> >>>> length,
> >>>>>> to be
> >>>>>>>> returned by a TS API function, like this:
> >>>>>>>>
> >>>>>>>> TSHeapBuf hb = TSSomething(x, y, z);
> >>>>>>>>
> >>>>>>>> One alternative is an interface like this:
> >>>>>>>>
> >>>>>>>> int length;
> >>>>>>>> char *data = TSSomething(x, y, z, &length);
> >>>>>>>>
> >>>>>>>> The data is dynamically allocated, and would be freed with
> >>>> TSfree().
> >>>>>>>>
> >>>>>>>> Another alternative is:
> >>>>>>>>
> >>>>>>>> char *buf = TSalloc(BUF_SIZE);
> >>>>>>>> int actual_size = TSSomething(x, y, z, buf, BUF_SIZE);
> >>>>>>>> if (actual_size > BUF_SIZE) {
> >>>>>>>> // buf was too small, unchanged.
> >>>>>>>> TSfree(buf);
> >>>>>>>> buf = TSalloc(actual_size);
> >>>>>>>> TSSomething(x, y, z, buf, actual_size);
> >>>>>>>> }
> >>>>>>>>
> >>>>>>
> >>>>>
> >>>>
> >>
> >>
>
>
Re: [E] Proposed change to TS API: TSHeapBuf
Posted by Leif Hedstrom <zw...@apache.org>.
> On Jan 4, 2023, at 3:03 PM, Walt Karas <wk...@yahooinc.com.INVALID> wrote:
>
> There are people at Yahoo who have written plugins in straight C, and don't
> want to change to C++.
They can continue to write plugins in C, just have to compile with a C++ compiler. I imagine there will be very few, if any changes. Of course, they can’t use any new APIs, or new structures added that are C++.
— leif
>
> On Wed, Jan 4, 2023 at 1:10 PM Leif Hedstrom <zw...@apache.org> wrote:
>
>> This looks like a string_view to me?
>>
>> Why don’t we do what we talked about for years now, and stop supporting
>> compiling plug-ins with C but require C++? Then we can introduce nice
>> things in ts.h. The old C plugins just have to be changed to compile with
>> C++, and ABI compatibility within major version must be enforced.
>>
>> — Leif
>>
>>> On Sep 6, 2022, at 10:24, Walt Karas <wk...@yahooinc.com.invalid>
>> wrote:
>>>
>>> Presumably we don't need to allow for use of antiquated C compilers that
>>> don't allow structures as return values. So this:
>>>
>>> typedef struct
>>> {
>>> char *data;
>>> int length;
>>> }
>>> TSVarLenData;
>>>
>>> TSVarLenData TSSomething(x, y, z);
>>>
>>> Is another alternative.
>>>
>>>> On Thu, Sep 1, 2022 at 5:32 PM Masakazu Kitajo <ma...@apache.org>
>> wrote:
>>>>
>>>> Using encapsulation/data abstraction is fine, but it doesn't seem like
>>>> TSHeapBuf makes sense. No TS API receives it. If plugin code is the only
>>>> user and it has to deal with a non-const raw pointer and a data length
>>>> after all, why do we want to wrap them in the first place?
>>>>
>>>> As for smaller steps, like I suggested on the PR, you could introduce
>>>> TSHeapBuf separately from the fix, I think. And if there are similar TS
>>>> APIs that could return TSHeapBuf, supporting TSHeapBuf on only one of
>> them
>>>> makes inconsistency in TS API. IMO, the two changes, the fix and the new
>>>> API, should be made like 1 + 1, but not 1.5 + 0.5 nor 1 + 0.5 + 0.5.
>>>>
>>>> Masakazu
>>>>
>>>> On Thu, Sep 1, 2022 at 10:08 AM Walt Karas <wkaras@yahooinc.com.invalid
>>>
>>>> wrote:
>>>>
>>>>> As a rule of thumb, I prefer using encapsulation/data abstraction. I
>>>> think
>>>>> perhaps that is one reason I've been a poor match to this project.
>> There
>>>>> doesn't seem to be a consensus that we should follow this rule of
>> thumb.
>>>>>
>>>>> KIt, that would be my preference. But I am part of the consensus I
>> think
>>>>> we have, that we should favor a series of smaller steps, rather than
>>>> doing
>>>>> all of them in one big step.
>>>>>
>>>>> On Wed, Aug 31, 2022 at 12:13 PM Shu Kit Chan <ch...@gmail.com>
>>>>> wrote:
>>>>>
>>>>>> Also are we planning to eventually rewrite our existing APIs (where
>>>>>> applicable) to use this?
>>>>>>
>>>>>> On Wed, Aug 31, 2022 at 8:36 AM Masakazu Kitajo <ma...@apache.org>
>>>>> wrote:
>>>>>>>
>>>>>>> What's the advantage of using TSHeapBuf? What issue does it solve?
>>>>>>>
>>>>>>>
>>>>>>> On Wed, Aug 31, 2022 at 7:48 AM Walt Karas
>>>> <wkaras@yahooinc.com.invalid
>>>>>>
>>>>>>> wrote:
>>>>>>>
>>>>>>>> Described here:
>>>>>>>>
>>>>>>>>
>>>>>>
>>>>>
>>>>
>> https://urldefense.com/v3/__https://github.com/apache/trafficserver/blob/os_pkey_cnf_reload/doc/developer-guide/api/functions/TSHeapBuf.en.rst*tsheapbufdata__;Iw!!Op6eflyXZCqGR5I!Du0fBfMhb4pdM2ECFijJ7aJ-jT70jEPeZwjhsvWt2Dr2cSZ5G7HWY20wZOmFHIR3MxnvPZpoRDMlII5dgow$
>>>>>>
>>>>>>>> ,
>>>>>>>>
>>>>>>>> In PR
>>>>>>
>>>>>
>>>>
>> https://urldefense.com/v3/__https://github.com/apache/trafficserver/pull/8790__;!!Op6eflyXZCqGR5I!Du0fBfMhb4pdM2ECFijJ7aJ-jT70jEPeZwjhsvWt2Dr2cSZ5G7HWY20wZOmFHIR3MxnvPZpoRDMlYh9lIuc$
>>>>>> .
>>>>>>>>
>>>>>>>> This allows a dynamically allocated buffer, of any reasonable
>>>> length,
>>>>>> to be
>>>>>>>> returned by a TS API function, like this:
>>>>>>>>
>>>>>>>> TSHeapBuf hb = TSSomething(x, y, z);
>>>>>>>>
>>>>>>>> One alternative is an interface like this:
>>>>>>>>
>>>>>>>> int length;
>>>>>>>> char *data = TSSomething(x, y, z, &length);
>>>>>>>>
>>>>>>>> The data is dynamically allocated, and would be freed with
>>>> TSfree().
>>>>>>>>
>>>>>>>> Another alternative is:
>>>>>>>>
>>>>>>>> char *buf = TSalloc(BUF_SIZE);
>>>>>>>> int actual_size = TSSomething(x, y, z, buf, BUF_SIZE);
>>>>>>>> if (actual_size > BUF_SIZE) {
>>>>>>>> // buf was too small, unchanged.
>>>>>>>> TSfree(buf);
>>>>>>>> buf = TSalloc(actual_size);
>>>>>>>> TSSomething(x, y, z, buf, actual_size);
>>>>>>>> }
>>>>>>>>
>>>>>>
>>>>>
>>>>
>>
>>
Re: [E] Re: Proposed change to TS API: TSHeapBuf
Posted by Walt Karas <wk...@yahooinc.com.INVALID>.
There are people at Yahoo who have written plugins in straight C, and don't
want to change to C++.
On Wed, Jan 4, 2023 at 1:10 PM Leif Hedstrom <zw...@apache.org> wrote:
> This looks like a string_view to me?
>
> Why don’t we do what we talked about for years now, and stop supporting
> compiling plug-ins with C but require C++? Then we can introduce nice
> things in ts.h. The old C plugins just have to be changed to compile with
> C++, and ABI compatibility within major version must be enforced.
>
> — Leif
>
> > On Sep 6, 2022, at 10:24, Walt Karas <wk...@yahooinc.com.invalid>
> wrote:
> >
> > Presumably we don't need to allow for use of antiquated C compilers that
> > don't allow structures as return values. So this:
> >
> > typedef struct
> > {
> > char *data;
> > int length;
> > }
> > TSVarLenData;
> >
> > TSVarLenData TSSomething(x, y, z);
> >
> > Is another alternative.
> >
> >> On Thu, Sep 1, 2022 at 5:32 PM Masakazu Kitajo <ma...@apache.org>
> wrote:
> >>
> >> Using encapsulation/data abstraction is fine, but it doesn't seem like
> >> TSHeapBuf makes sense. No TS API receives it. If plugin code is the only
> >> user and it has to deal with a non-const raw pointer and a data length
> >> after all, why do we want to wrap them in the first place?
> >>
> >> As for smaller steps, like I suggested on the PR, you could introduce
> >> TSHeapBuf separately from the fix, I think. And if there are similar TS
> >> APIs that could return TSHeapBuf, supporting TSHeapBuf on only one of
> them
> >> makes inconsistency in TS API. IMO, the two changes, the fix and the new
> >> API, should be made like 1 + 1, but not 1.5 + 0.5 nor 1 + 0.5 + 0.5.
> >>
> >> Masakazu
> >>
> >> On Thu, Sep 1, 2022 at 10:08 AM Walt Karas <wkaras@yahooinc.com.invalid
> >
> >> wrote:
> >>
> >>> As a rule of thumb, I prefer using encapsulation/data abstraction. I
> >> think
> >>> perhaps that is one reason I've been a poor match to this project.
> There
> >>> doesn't seem to be a consensus that we should follow this rule of
> thumb.
> >>>
> >>> KIt, that would be my preference. But I am part of the consensus I
> think
> >>> we have, that we should favor a series of smaller steps, rather than
> >> doing
> >>> all of them in one big step.
> >>>
> >>> On Wed, Aug 31, 2022 at 12:13 PM Shu Kit Chan <ch...@gmail.com>
> >>> wrote:
> >>>
> >>>> Also are we planning to eventually rewrite our existing APIs (where
> >>>> applicable) to use this?
> >>>>
> >>>> On Wed, Aug 31, 2022 at 8:36 AM Masakazu Kitajo <ma...@apache.org>
> >>> wrote:
> >>>>>
> >>>>> What's the advantage of using TSHeapBuf? What issue does it solve?
> >>>>>
> >>>>>
> >>>>> On Wed, Aug 31, 2022 at 7:48 AM Walt Karas
> >> <wkaras@yahooinc.com.invalid
> >>>>
> >>>>> wrote:
> >>>>>
> >>>>>> Described here:
> >>>>>>
> >>>>>>
> >>>>
> >>>
> >>
> https://urldefense.com/v3/__https://github.com/apache/trafficserver/blob/os_pkey_cnf_reload/doc/developer-guide/api/functions/TSHeapBuf.en.rst*tsheapbufdata__;Iw!!Op6eflyXZCqGR5I!Du0fBfMhb4pdM2ECFijJ7aJ-jT70jEPeZwjhsvWt2Dr2cSZ5G7HWY20wZOmFHIR3MxnvPZpoRDMlII5dgow$
> >>>>
> >>>>>> ,
> >>>>>>
> >>>>>> In PR
> >>>>
> >>>
> >>
> https://urldefense.com/v3/__https://github.com/apache/trafficserver/pull/8790__;!!Op6eflyXZCqGR5I!Du0fBfMhb4pdM2ECFijJ7aJ-jT70jEPeZwjhsvWt2Dr2cSZ5G7HWY20wZOmFHIR3MxnvPZpoRDMlYh9lIuc$
> >>>> .
> >>>>>>
> >>>>>> This allows a dynamically allocated buffer, of any reasonable
> >> length,
> >>>> to be
> >>>>>> returned by a TS API function, like this:
> >>>>>>
> >>>>>> TSHeapBuf hb = TSSomething(x, y, z);
> >>>>>>
> >>>>>> One alternative is an interface like this:
> >>>>>>
> >>>>>> int length;
> >>>>>> char *data = TSSomething(x, y, z, &length);
> >>>>>>
> >>>>>> The data is dynamically allocated, and would be freed with
> >> TSfree().
> >>>>>>
> >>>>>> Another alternative is:
> >>>>>>
> >>>>>> char *buf = TSalloc(BUF_SIZE);
> >>>>>> int actual_size = TSSomething(x, y, z, buf, BUF_SIZE);
> >>>>>> if (actual_size > BUF_SIZE) {
> >>>>>> // buf was too small, unchanged.
> >>>>>> TSfree(buf);
> >>>>>> buf = TSalloc(actual_size);
> >>>>>> TSSomething(x, y, z, buf, actual_size);
> >>>>>> }
> >>>>>>
> >>>>
> >>>
> >>
>
>
Re: [E] Re: Proposed change to TS API: TSHeapBuf
Posted by Leif Hedstrom <zw...@apache.org>.
This looks like a string_view to me?
Why don’t we do what we talked about for years now, and stop supporting compiling plug-ins with C but require C++? Then we can introduce nice things in ts.h. The old C plugins just have to be changed to compile with C++, and ABI compatibility within major version must be enforced.
— Leif
> On Sep 6, 2022, at 10:24, Walt Karas <wk...@yahooinc.com.invalid> wrote:
>
> Presumably we don't need to allow for use of antiquated C compilers that
> don't allow structures as return values. So this:
>
> typedef struct
> {
> char *data;
> int length;
> }
> TSVarLenData;
>
> TSVarLenData TSSomething(x, y, z);
>
> Is another alternative.
>
>> On Thu, Sep 1, 2022 at 5:32 PM Masakazu Kitajo <ma...@apache.org> wrote:
>>
>> Using encapsulation/data abstraction is fine, but it doesn't seem like
>> TSHeapBuf makes sense. No TS API receives it. If plugin code is the only
>> user and it has to deal with a non-const raw pointer and a data length
>> after all, why do we want to wrap them in the first place?
>>
>> As for smaller steps, like I suggested on the PR, you could introduce
>> TSHeapBuf separately from the fix, I think. And if there are similar TS
>> APIs that could return TSHeapBuf, supporting TSHeapBuf on only one of them
>> makes inconsistency in TS API. IMO, the two changes, the fix and the new
>> API, should be made like 1 + 1, but not 1.5 + 0.5 nor 1 + 0.5 + 0.5.
>>
>> Masakazu
>>
>> On Thu, Sep 1, 2022 at 10:08 AM Walt Karas <wk...@yahooinc.com.invalid>
>> wrote:
>>
>>> As a rule of thumb, I prefer using encapsulation/data abstraction. I
>> think
>>> perhaps that is one reason I've been a poor match to this project. There
>>> doesn't seem to be a consensus that we should follow this rule of thumb.
>>>
>>> KIt, that would be my preference. But I am part of the consensus I think
>>> we have, that we should favor a series of smaller steps, rather than
>> doing
>>> all of them in one big step.
>>>
>>> On Wed, Aug 31, 2022 at 12:13 PM Shu Kit Chan <ch...@gmail.com>
>>> wrote:
>>>
>>>> Also are we planning to eventually rewrite our existing APIs (where
>>>> applicable) to use this?
>>>>
>>>> On Wed, Aug 31, 2022 at 8:36 AM Masakazu Kitajo <ma...@apache.org>
>>> wrote:
>>>>>
>>>>> What's the advantage of using TSHeapBuf? What issue does it solve?
>>>>>
>>>>>
>>>>> On Wed, Aug 31, 2022 at 7:48 AM Walt Karas
>> <wkaras@yahooinc.com.invalid
>>>>
>>>>> wrote:
>>>>>
>>>>>> Described here:
>>>>>>
>>>>>>
>>>>
>>>
>> https://urldefense.com/v3/__https://github.com/apache/trafficserver/blob/os_pkey_cnf_reload/doc/developer-guide/api/functions/TSHeapBuf.en.rst*tsheapbufdata__;Iw!!Op6eflyXZCqGR5I!Du0fBfMhb4pdM2ECFijJ7aJ-jT70jEPeZwjhsvWt2Dr2cSZ5G7HWY20wZOmFHIR3MxnvPZpoRDMlII5dgow$
>>>>
>>>>>> ,
>>>>>>
>>>>>> In PR
>>>>
>>>
>> https://urldefense.com/v3/__https://github.com/apache/trafficserver/pull/8790__;!!Op6eflyXZCqGR5I!Du0fBfMhb4pdM2ECFijJ7aJ-jT70jEPeZwjhsvWt2Dr2cSZ5G7HWY20wZOmFHIR3MxnvPZpoRDMlYh9lIuc$
>>>> .
>>>>>>
>>>>>> This allows a dynamically allocated buffer, of any reasonable
>> length,
>>>> to be
>>>>>> returned by a TS API function, like this:
>>>>>>
>>>>>> TSHeapBuf hb = TSSomething(x, y, z);
>>>>>>
>>>>>> One alternative is an interface like this:
>>>>>>
>>>>>> int length;
>>>>>> char *data = TSSomething(x, y, z, &length);
>>>>>>
>>>>>> The data is dynamically allocated, and would be freed with
>> TSfree().
>>>>>>
>>>>>> Another alternative is:
>>>>>>
>>>>>> char *buf = TSalloc(BUF_SIZE);
>>>>>> int actual_size = TSSomething(x, y, z, buf, BUF_SIZE);
>>>>>> if (actual_size > BUF_SIZE) {
>>>>>> // buf was too small, unchanged.
>>>>>> TSfree(buf);
>>>>>> buf = TSalloc(actual_size);
>>>>>> TSSomething(x, y, z, buf, actual_size);
>>>>>> }
>>>>>>
>>>>
>>>
>>
Re: [E] Re: Proposed change to TS API: TSHeapBuf
Posted by Walt Karas <wk...@yahooinc.com.INVALID>.
I'll wait a couple more days for input, then I'll change it to return char
* and take an int * parameter to output the length. If we're switching to
C++, we can update this based on whatever consensus we reach for the
switchover strategy.
On Wed, Jan 4, 2023 at 7:03 PM Masakazu Kitajo <ma...@apache.org> wrote:
> > They could make the returned structure a nesting structure in
> their structure.
>
> What if the structure is defined by a library and a plugin author can't
> modify it? What I'm trying to say is that I don't see the benefit of
> limiting how a user receives the pointer and the length where nothing
> receives TSVarLenData. Do we require two copy ops for the "unusual" case
> just to avoid having two variables by providing TSVarLenData? Is it that
> convenient?
>
> I don't have a strong opinion about moving to C++ at the moment, but if we
> go that way and start using string_view, TSVarLenData wouldn't be used for
> new APIs. And we'd add a string_view version of an old API that uses
> TSVarLenData so that users can easily pass the returned value to other
> functions. Then we'd deprecate the old API and remove it in the future.
> That's a lot of work.
>
> I feel like introducing TSVarLenData has more downside than the upside.
>
> On Wed, Jan 4, 2023 at 9:13 AM Walt Karas <wk...@yahooinc.com.invalid>
> wrote:
>
> > They could make the returned structure a nesting structure in their
> > structure. Better to make things more convenient for the typical case,
> not
> > the unusual case.
> >
> > On Tue, Jan 3, 2023 at 12:28 PM Masakazu Kitajo <ma...@apache.org>
> wrote:
> >
> > > What's the benefit of forcing users to use the structure? A user might
> > want
> > > to store the returned pointer into a user-defined structure.
> > >
> > >
> > >
> > > On Tue, Sep 6, 2022 at 10:24 AM Walt Karas <wkaras@yahooinc.com.invalid
> >
> > > wrote:
> > >
> > > > Presumably we don't need to allow for use of antiquated C compilers
> > that
> > > > don't allow structures as return values. So this:
> > > >
> > > > typedef struct
> > > > {
> > > > char *data;
> > > > int length;
> > > > }
> > > > TSVarLenData;
> > > >
> > > > TSVarLenData TSSomething(x, y, z);
> > > >
> > > > Is another alternative.
> > > >
> > > > On Thu, Sep 1, 2022 at 5:32 PM Masakazu Kitajo <ma...@apache.org>
> > > wrote:
> > > >
> > > > > Using encapsulation/data abstraction is fine, but it doesn't seem
> > like
> > > > > TSHeapBuf makes sense. No TS API receives it. If plugin code is the
> > > only
> > > > > user and it has to deal with a non-const raw pointer and a data
> > length
> > > > > after all, why do we want to wrap them in the first place?
> > > > >
> > > > > As for smaller steps, like I suggested on the PR, you could
> introduce
> > > > > TSHeapBuf separately from the fix, I think. And if there are
> similar
> > TS
> > > > > APIs that could return TSHeapBuf, supporting TSHeapBuf on only one
> of
> > > > them
> > > > > makes inconsistency in TS API. IMO, the two changes, the fix and
> the
> > > new
> > > > > API, should be made like 1 + 1, but not 1.5 + 0.5 nor 1 + 0.5 +
> 0.5.
> > > > >
> > > > > Masakazu
> > > > >
> > > > > On Thu, Sep 1, 2022 at 10:08 AM Walt Karas
> > <wkaras@yahooinc.com.invalid
> > > >
> > > > > wrote:
> > > > >
> > > > > > As a rule of thumb, I prefer using encapsulation/data
> > abstraction. I
> > > > > think
> > > > > > perhaps that is one reason I've been a poor match to this
> project.
> > > > There
> > > > > > doesn't seem to be a consensus that we should follow this rule
> of
> > > > thumb.
> > > > > >
> > > > > > KIt, that would be my preference. But I am part of the
> consensus I
> > > > think
> > > > > > we have, that we should favor a series of smaller steps, rather
> > than
> > > > > doing
> > > > > > all of them in one big step.
> > > > > >
> > > > > > On Wed, Aug 31, 2022 at 12:13 PM Shu Kit Chan <
> > chanshukit@gmail.com>
> > > > > > wrote:
> > > > > >
> > > > > > > Also are we planning to eventually rewrite our existing APIs
> > (where
> > > > > > > applicable) to use this?
> > > > > > >
> > > > > > > On Wed, Aug 31, 2022 at 8:36 AM Masakazu Kitajo <
> > maskit@apache.org
> > > >
> > > > > > wrote:
> > > > > > > >
> > > > > > > > What's the advantage of using TSHeapBuf? What issue does it
> > > solve?
> > > > > > > >
> > > > > > > >
> > > > > > > > On Wed, Aug 31, 2022 at 7:48 AM Walt Karas
> > > > > <wkaras@yahooinc.com.invalid
> > > > > > >
> > > > > > > > wrote:
> > > > > > > >
> > > > > > > > > Described here:
> > > > > > > > >
> > > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
> https://urldefense.com/v3/__https://github.com/apache/trafficserver/blob/os_pkey_cnf_reload/doc/developer-guide/api/functions/TSHeapBuf.en.rst*tsheapbufdata__;Iw!!Op6eflyXZCqGR5I!Du0fBfMhb4pdM2ECFijJ7aJ-jT70jEPeZwjhsvWt2Dr2cSZ5G7HWY20wZOmFHIR3MxnvPZpoRDMlII5dgow$
> > > > > > >
> > > > > > > > > ,
> > > > > > > > >
> > > > > > > > > In PR
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
> https://urldefense.com/v3/__https://github.com/apache/trafficserver/pull/8790__;!!Op6eflyXZCqGR5I!Du0fBfMhb4pdM2ECFijJ7aJ-jT70jEPeZwjhsvWt2Dr2cSZ5G7HWY20wZOmFHIR3MxnvPZpoRDMlYh9lIuc$
> > > > > > > .
> > > > > > > > >
> > > > > > > > > This allows a dynamically allocated buffer, of any
> reasonable
> > > > > length,
> > > > > > > to be
> > > > > > > > > returned by a TS API function, like this:
> > > > > > > > >
> > > > > > > > > TSHeapBuf hb = TSSomething(x, y, z);
> > > > > > > > >
> > > > > > > > > One alternative is an interface like this:
> > > > > > > > >
> > > > > > > > > int length;
> > > > > > > > > char *data = TSSomething(x, y, z, &length);
> > > > > > > > >
> > > > > > > > > The data is dynamically allocated, and would be freed with
> > > > > TSfree().
> > > > > > > > >
> > > > > > > > > Another alternative is:
> > > > > > > > >
> > > > > > > > > char *buf = TSalloc(BUF_SIZE);
> > > > > > > > > int actual_size = TSSomething(x, y, z, buf, BUF_SIZE);
> > > > > > > > > if (actual_size > BUF_SIZE) {
> > > > > > > > > // buf was too small, unchanged.
> > > > > > > > > TSfree(buf);
> > > > > > > > > buf = TSalloc(actual_size);
> > > > > > > > > TSSomething(x, y, z, buf, actual_size);
> > > > > > > > > }
> > > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
Re: [E] Re: Proposed change to TS API: TSHeapBuf
Posted by Masakazu Kitajo <ma...@apache.org>.
> They could make the returned structure a nesting structure in
their structure.
What if the structure is defined by a library and a plugin author can't
modify it? What I'm trying to say is that I don't see the benefit of
limiting how a user receives the pointer and the length where nothing
receives TSVarLenData. Do we require two copy ops for the "unusual" case
just to avoid having two variables by providing TSVarLenData? Is it that
convenient?
I don't have a strong opinion about moving to C++ at the moment, but if we
go that way and start using string_view, TSVarLenData wouldn't be used for
new APIs. And we'd add a string_view version of an old API that uses
TSVarLenData so that users can easily pass the returned value to other
functions. Then we'd deprecate the old API and remove it in the future.
That's a lot of work.
I feel like introducing TSVarLenData has more downside than the upside.
On Wed, Jan 4, 2023 at 9:13 AM Walt Karas <wk...@yahooinc.com.invalid>
wrote:
> They could make the returned structure a nesting structure in their
> structure. Better to make things more convenient for the typical case, not
> the unusual case.
>
> On Tue, Jan 3, 2023 at 12:28 PM Masakazu Kitajo <ma...@apache.org> wrote:
>
> > What's the benefit of forcing users to use the structure? A user might
> want
> > to store the returned pointer into a user-defined structure.
> >
> >
> >
> > On Tue, Sep 6, 2022 at 10:24 AM Walt Karas <wk...@yahooinc.com.invalid>
> > wrote:
> >
> > > Presumably we don't need to allow for use of antiquated C compilers
> that
> > > don't allow structures as return values. So this:
> > >
> > > typedef struct
> > > {
> > > char *data;
> > > int length;
> > > }
> > > TSVarLenData;
> > >
> > > TSVarLenData TSSomething(x, y, z);
> > >
> > > Is another alternative.
> > >
> > > On Thu, Sep 1, 2022 at 5:32 PM Masakazu Kitajo <ma...@apache.org>
> > wrote:
> > >
> > > > Using encapsulation/data abstraction is fine, but it doesn't seem
> like
> > > > TSHeapBuf makes sense. No TS API receives it. If plugin code is the
> > only
> > > > user and it has to deal with a non-const raw pointer and a data
> length
> > > > after all, why do we want to wrap them in the first place?
> > > >
> > > > As for smaller steps, like I suggested on the PR, you could introduce
> > > > TSHeapBuf separately from the fix, I think. And if there are similar
> TS
> > > > APIs that could return TSHeapBuf, supporting TSHeapBuf on only one of
> > > them
> > > > makes inconsistency in TS API. IMO, the two changes, the fix and the
> > new
> > > > API, should be made like 1 + 1, but not 1.5 + 0.5 nor 1 + 0.5 + 0.5.
> > > >
> > > > Masakazu
> > > >
> > > > On Thu, Sep 1, 2022 at 10:08 AM Walt Karas
> <wkaras@yahooinc.com.invalid
> > >
> > > > wrote:
> > > >
> > > > > As a rule of thumb, I prefer using encapsulation/data
> abstraction. I
> > > > think
> > > > > perhaps that is one reason I've been a poor match to this project.
> > > There
> > > > > doesn't seem to be a consensus that we should follow this rule of
> > > thumb.
> > > > >
> > > > > KIt, that would be my preference. But I am part of the consensus I
> > > think
> > > > > we have, that we should favor a series of smaller steps, rather
> than
> > > > doing
> > > > > all of them in one big step.
> > > > >
> > > > > On Wed, Aug 31, 2022 at 12:13 PM Shu Kit Chan <
> chanshukit@gmail.com>
> > > > > wrote:
> > > > >
> > > > > > Also are we planning to eventually rewrite our existing APIs
> (where
> > > > > > applicable) to use this?
> > > > > >
> > > > > > On Wed, Aug 31, 2022 at 8:36 AM Masakazu Kitajo <
> maskit@apache.org
> > >
> > > > > wrote:
> > > > > > >
> > > > > > > What's the advantage of using TSHeapBuf? What issue does it
> > solve?
> > > > > > >
> > > > > > >
> > > > > > > On Wed, Aug 31, 2022 at 7:48 AM Walt Karas
> > > > <wkaras@yahooinc.com.invalid
> > > > > >
> > > > > > > wrote:
> > > > > > >
> > > > > > > > Described here:
> > > > > > > >
> > > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
> https://urldefense.com/v3/__https://github.com/apache/trafficserver/blob/os_pkey_cnf_reload/doc/developer-guide/api/functions/TSHeapBuf.en.rst*tsheapbufdata__;Iw!!Op6eflyXZCqGR5I!Du0fBfMhb4pdM2ECFijJ7aJ-jT70jEPeZwjhsvWt2Dr2cSZ5G7HWY20wZOmFHIR3MxnvPZpoRDMlII5dgow$
> > > > > >
> > > > > > > > ,
> > > > > > > >
> > > > > > > > In PR
> > > > > >
> > > > >
> > > >
> > >
> >
> https://urldefense.com/v3/__https://github.com/apache/trafficserver/pull/8790__;!!Op6eflyXZCqGR5I!Du0fBfMhb4pdM2ECFijJ7aJ-jT70jEPeZwjhsvWt2Dr2cSZ5G7HWY20wZOmFHIR3MxnvPZpoRDMlYh9lIuc$
> > > > > > .
> > > > > > > >
> > > > > > > > This allows a dynamically allocated buffer, of any reasonable
> > > > length,
> > > > > > to be
> > > > > > > > returned by a TS API function, like this:
> > > > > > > >
> > > > > > > > TSHeapBuf hb = TSSomething(x, y, z);
> > > > > > > >
> > > > > > > > One alternative is an interface like this:
> > > > > > > >
> > > > > > > > int length;
> > > > > > > > char *data = TSSomething(x, y, z, &length);
> > > > > > > >
> > > > > > > > The data is dynamically allocated, and would be freed with
> > > > TSfree().
> > > > > > > >
> > > > > > > > Another alternative is:
> > > > > > > >
> > > > > > > > char *buf = TSalloc(BUF_SIZE);
> > > > > > > > int actual_size = TSSomething(x, y, z, buf, BUF_SIZE);
> > > > > > > > if (actual_size > BUF_SIZE) {
> > > > > > > > // buf was too small, unchanged.
> > > > > > > > TSfree(buf);
> > > > > > > > buf = TSalloc(actual_size);
> > > > > > > > TSSomething(x, y, z, buf, actual_size);
> > > > > > > > }
> > > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
Re: [E] Re: Proposed change to TS API: TSHeapBuf
Posted by Walt Karas <wk...@yahooinc.com.INVALID>.
They could make the returned structure a nesting structure in their
structure. Better to make things more convenient for the typical case, not
the unusual case.
On Tue, Jan 3, 2023 at 12:28 PM Masakazu Kitajo <ma...@apache.org> wrote:
> What's the benefit of forcing users to use the structure? A user might want
> to store the returned pointer into a user-defined structure.
>
>
>
> On Tue, Sep 6, 2022 at 10:24 AM Walt Karas <wk...@yahooinc.com.invalid>
> wrote:
>
> > Presumably we don't need to allow for use of antiquated C compilers that
> > don't allow structures as return values. So this:
> >
> > typedef struct
> > {
> > char *data;
> > int length;
> > }
> > TSVarLenData;
> >
> > TSVarLenData TSSomething(x, y, z);
> >
> > Is another alternative.
> >
> > On Thu, Sep 1, 2022 at 5:32 PM Masakazu Kitajo <ma...@apache.org>
> wrote:
> >
> > > Using encapsulation/data abstraction is fine, but it doesn't seem like
> > > TSHeapBuf makes sense. No TS API receives it. If plugin code is the
> only
> > > user and it has to deal with a non-const raw pointer and a data length
> > > after all, why do we want to wrap them in the first place?
> > >
> > > As for smaller steps, like I suggested on the PR, you could introduce
> > > TSHeapBuf separately from the fix, I think. And if there are similar TS
> > > APIs that could return TSHeapBuf, supporting TSHeapBuf on only one of
> > them
> > > makes inconsistency in TS API. IMO, the two changes, the fix and the
> new
> > > API, should be made like 1 + 1, but not 1.5 + 0.5 nor 1 + 0.5 + 0.5.
> > >
> > > Masakazu
> > >
> > > On Thu, Sep 1, 2022 at 10:08 AM Walt Karas <wkaras@yahooinc.com.invalid
> >
> > > wrote:
> > >
> > > > As a rule of thumb, I prefer using encapsulation/data abstraction. I
> > > think
> > > > perhaps that is one reason I've been a poor match to this project.
> > There
> > > > doesn't seem to be a consensus that we should follow this rule of
> > thumb.
> > > >
> > > > KIt, that would be my preference. But I am part of the consensus I
> > think
> > > > we have, that we should favor a series of smaller steps, rather than
> > > doing
> > > > all of them in one big step.
> > > >
> > > > On Wed, Aug 31, 2022 at 12:13 PM Shu Kit Chan <ch...@gmail.com>
> > > > wrote:
> > > >
> > > > > Also are we planning to eventually rewrite our existing APIs (where
> > > > > applicable) to use this?
> > > > >
> > > > > On Wed, Aug 31, 2022 at 8:36 AM Masakazu Kitajo <maskit@apache.org
> >
> > > > wrote:
> > > > > >
> > > > > > What's the advantage of using TSHeapBuf? What issue does it
> solve?
> > > > > >
> > > > > >
> > > > > > On Wed, Aug 31, 2022 at 7:48 AM Walt Karas
> > > <wkaras@yahooinc.com.invalid
> > > > >
> > > > > > wrote:
> > > > > >
> > > > > > > Described here:
> > > > > > >
> > > > > > >
> > > > >
> > > >
> > >
> >
> https://urldefense.com/v3/__https://github.com/apache/trafficserver/blob/os_pkey_cnf_reload/doc/developer-guide/api/functions/TSHeapBuf.en.rst*tsheapbufdata__;Iw!!Op6eflyXZCqGR5I!Du0fBfMhb4pdM2ECFijJ7aJ-jT70jEPeZwjhsvWt2Dr2cSZ5G7HWY20wZOmFHIR3MxnvPZpoRDMlII5dgow$
> > > > >
> > > > > > > ,
> > > > > > >
> > > > > > > In PR
> > > > >
> > > >
> > >
> >
> https://urldefense.com/v3/__https://github.com/apache/trafficserver/pull/8790__;!!Op6eflyXZCqGR5I!Du0fBfMhb4pdM2ECFijJ7aJ-jT70jEPeZwjhsvWt2Dr2cSZ5G7HWY20wZOmFHIR3MxnvPZpoRDMlYh9lIuc$
> > > > > .
> > > > > > >
> > > > > > > This allows a dynamically allocated buffer, of any reasonable
> > > length,
> > > > > to be
> > > > > > > returned by a TS API function, like this:
> > > > > > >
> > > > > > > TSHeapBuf hb = TSSomething(x, y, z);
> > > > > > >
> > > > > > > One alternative is an interface like this:
> > > > > > >
> > > > > > > int length;
> > > > > > > char *data = TSSomething(x, y, z, &length);
> > > > > > >
> > > > > > > The data is dynamically allocated, and would be freed with
> > > TSfree().
> > > > > > >
> > > > > > > Another alternative is:
> > > > > > >
> > > > > > > char *buf = TSalloc(BUF_SIZE);
> > > > > > > int actual_size = TSSomething(x, y, z, buf, BUF_SIZE);
> > > > > > > if (actual_size > BUF_SIZE) {
> > > > > > > // buf was too small, unchanged.
> > > > > > > TSfree(buf);
> > > > > > > buf = TSalloc(actual_size);
> > > > > > > TSSomething(x, y, z, buf, actual_size);
> > > > > > > }
> > > > > > >
> > > > >
> > > >
> > >
> >
>
Re: [E] Re: Proposed change to TS API: TSHeapBuf
Posted by Masakazu Kitajo <ma...@apache.org>.
What's the benefit of forcing users to use the structure? A user might want
to store the returned pointer into a user-defined structure.
On Tue, Sep 6, 2022 at 10:24 AM Walt Karas <wk...@yahooinc.com.invalid>
wrote:
> Presumably we don't need to allow for use of antiquated C compilers that
> don't allow structures as return values. So this:
>
> typedef struct
> {
> char *data;
> int length;
> }
> TSVarLenData;
>
> TSVarLenData TSSomething(x, y, z);
>
> Is another alternative.
>
> On Thu, Sep 1, 2022 at 5:32 PM Masakazu Kitajo <ma...@apache.org> wrote:
>
> > Using encapsulation/data abstraction is fine, but it doesn't seem like
> > TSHeapBuf makes sense. No TS API receives it. If plugin code is the only
> > user and it has to deal with a non-const raw pointer and a data length
> > after all, why do we want to wrap them in the first place?
> >
> > As for smaller steps, like I suggested on the PR, you could introduce
> > TSHeapBuf separately from the fix, I think. And if there are similar TS
> > APIs that could return TSHeapBuf, supporting TSHeapBuf on only one of
> them
> > makes inconsistency in TS API. IMO, the two changes, the fix and the new
> > API, should be made like 1 + 1, but not 1.5 + 0.5 nor 1 + 0.5 + 0.5.
> >
> > Masakazu
> >
> > On Thu, Sep 1, 2022 at 10:08 AM Walt Karas <wk...@yahooinc.com.invalid>
> > wrote:
> >
> > > As a rule of thumb, I prefer using encapsulation/data abstraction. I
> > think
> > > perhaps that is one reason I've been a poor match to this project.
> There
> > > doesn't seem to be a consensus that we should follow this rule of
> thumb.
> > >
> > > KIt, that would be my preference. But I am part of the consensus I
> think
> > > we have, that we should favor a series of smaller steps, rather than
> > doing
> > > all of them in one big step.
> > >
> > > On Wed, Aug 31, 2022 at 12:13 PM Shu Kit Chan <ch...@gmail.com>
> > > wrote:
> > >
> > > > Also are we planning to eventually rewrite our existing APIs (where
> > > > applicable) to use this?
> > > >
> > > > On Wed, Aug 31, 2022 at 8:36 AM Masakazu Kitajo <ma...@apache.org>
> > > wrote:
> > > > >
> > > > > What's the advantage of using TSHeapBuf? What issue does it solve?
> > > > >
> > > > >
> > > > > On Wed, Aug 31, 2022 at 7:48 AM Walt Karas
> > <wkaras@yahooinc.com.invalid
> > > >
> > > > > wrote:
> > > > >
> > > > > > Described here:
> > > > > >
> > > > > >
> > > >
> > >
> >
> https://urldefense.com/v3/__https://github.com/apache/trafficserver/blob/os_pkey_cnf_reload/doc/developer-guide/api/functions/TSHeapBuf.en.rst*tsheapbufdata__;Iw!!Op6eflyXZCqGR5I!Du0fBfMhb4pdM2ECFijJ7aJ-jT70jEPeZwjhsvWt2Dr2cSZ5G7HWY20wZOmFHIR3MxnvPZpoRDMlII5dgow$
> > > >
> > > > > > ,
> > > > > >
> > > > > > In PR
> > > >
> > >
> >
> https://urldefense.com/v3/__https://github.com/apache/trafficserver/pull/8790__;!!Op6eflyXZCqGR5I!Du0fBfMhb4pdM2ECFijJ7aJ-jT70jEPeZwjhsvWt2Dr2cSZ5G7HWY20wZOmFHIR3MxnvPZpoRDMlYh9lIuc$
> > > > .
> > > > > >
> > > > > > This allows a dynamically allocated buffer, of any reasonable
> > length,
> > > > to be
> > > > > > returned by a TS API function, like this:
> > > > > >
> > > > > > TSHeapBuf hb = TSSomething(x, y, z);
> > > > > >
> > > > > > One alternative is an interface like this:
> > > > > >
> > > > > > int length;
> > > > > > char *data = TSSomething(x, y, z, &length);
> > > > > >
> > > > > > The data is dynamically allocated, and would be freed with
> > TSfree().
> > > > > >
> > > > > > Another alternative is:
> > > > > >
> > > > > > char *buf = TSalloc(BUF_SIZE);
> > > > > > int actual_size = TSSomething(x, y, z, buf, BUF_SIZE);
> > > > > > if (actual_size > BUF_SIZE) {
> > > > > > // buf was too small, unchanged.
> > > > > > TSfree(buf);
> > > > > > buf = TSalloc(actual_size);
> > > > > > TSSomething(x, y, z, buf, actual_size);
> > > > > > }
> > > > > >
> > > >
> > >
> >
>
Re: [E] Re: Proposed change to TS API: TSHeapBuf
Posted by Walt Karas <wk...@yahooinc.com.INVALID>.
Presumably we don't need to allow for use of antiquated C compilers that
don't allow structures as return values. So this:
typedef struct
{
char *data;
int length;
}
TSVarLenData;
TSVarLenData TSSomething(x, y, z);
Is another alternative.
On Thu, Sep 1, 2022 at 5:32 PM Masakazu Kitajo <ma...@apache.org> wrote:
> Using encapsulation/data abstraction is fine, but it doesn't seem like
> TSHeapBuf makes sense. No TS API receives it. If plugin code is the only
> user and it has to deal with a non-const raw pointer and a data length
> after all, why do we want to wrap them in the first place?
>
> As for smaller steps, like I suggested on the PR, you could introduce
> TSHeapBuf separately from the fix, I think. And if there are similar TS
> APIs that could return TSHeapBuf, supporting TSHeapBuf on only one of them
> makes inconsistency in TS API. IMO, the two changes, the fix and the new
> API, should be made like 1 + 1, but not 1.5 + 0.5 nor 1 + 0.5 + 0.5.
>
> Masakazu
>
> On Thu, Sep 1, 2022 at 10:08 AM Walt Karas <wk...@yahooinc.com.invalid>
> wrote:
>
> > As a rule of thumb, I prefer using encapsulation/data abstraction. I
> think
> > perhaps that is one reason I've been a poor match to this project. There
> > doesn't seem to be a consensus that we should follow this rule of thumb.
> >
> > KIt, that would be my preference. But I am part of the consensus I think
> > we have, that we should favor a series of smaller steps, rather than
> doing
> > all of them in one big step.
> >
> > On Wed, Aug 31, 2022 at 12:13 PM Shu Kit Chan <ch...@gmail.com>
> > wrote:
> >
> > > Also are we planning to eventually rewrite our existing APIs (where
> > > applicable) to use this?
> > >
> > > On Wed, Aug 31, 2022 at 8:36 AM Masakazu Kitajo <ma...@apache.org>
> > wrote:
> > > >
> > > > What's the advantage of using TSHeapBuf? What issue does it solve?
> > > >
> > > >
> > > > On Wed, Aug 31, 2022 at 7:48 AM Walt Karas
> <wkaras@yahooinc.com.invalid
> > >
> > > > wrote:
> > > >
> > > > > Described here:
> > > > >
> > > > >
> > >
> >
> https://urldefense.com/v3/__https://github.com/apache/trafficserver/blob/os_pkey_cnf_reload/doc/developer-guide/api/functions/TSHeapBuf.en.rst*tsheapbufdata__;Iw!!Op6eflyXZCqGR5I!Du0fBfMhb4pdM2ECFijJ7aJ-jT70jEPeZwjhsvWt2Dr2cSZ5G7HWY20wZOmFHIR3MxnvPZpoRDMlII5dgow$
> > >
> > > > > ,
> > > > >
> > > > > In PR
> > >
> >
> https://urldefense.com/v3/__https://github.com/apache/trafficserver/pull/8790__;!!Op6eflyXZCqGR5I!Du0fBfMhb4pdM2ECFijJ7aJ-jT70jEPeZwjhsvWt2Dr2cSZ5G7HWY20wZOmFHIR3MxnvPZpoRDMlYh9lIuc$
> > > .
> > > > >
> > > > > This allows a dynamically allocated buffer, of any reasonable
> length,
> > > to be
> > > > > returned by a TS API function, like this:
> > > > >
> > > > > TSHeapBuf hb = TSSomething(x, y, z);
> > > > >
> > > > > One alternative is an interface like this:
> > > > >
> > > > > int length;
> > > > > char *data = TSSomething(x, y, z, &length);
> > > > >
> > > > > The data is dynamically allocated, and would be freed with
> TSfree().
> > > > >
> > > > > Another alternative is:
> > > > >
> > > > > char *buf = TSalloc(BUF_SIZE);
> > > > > int actual_size = TSSomething(x, y, z, buf, BUF_SIZE);
> > > > > if (actual_size > BUF_SIZE) {
> > > > > // buf was too small, unchanged.
> > > > > TSfree(buf);
> > > > > buf = TSalloc(actual_size);
> > > > > TSSomething(x, y, z, buf, actual_size);
> > > > > }
> > > > >
> > >
> >
>
Re: [E] Re: Proposed change to TS API: TSHeapBuf
Posted by Masakazu Kitajo <ma...@apache.org>.
Using encapsulation/data abstraction is fine, but it doesn't seem like
TSHeapBuf makes sense. No TS API receives it. If plugin code is the only
user and it has to deal with a non-const raw pointer and a data length
after all, why do we want to wrap them in the first place?
As for smaller steps, like I suggested on the PR, you could introduce
TSHeapBuf separately from the fix, I think. And if there are similar TS
APIs that could return TSHeapBuf, supporting TSHeapBuf on only one of them
makes inconsistency in TS API. IMO, the two changes, the fix and the new
API, should be made like 1 + 1, but not 1.5 + 0.5 nor 1 + 0.5 + 0.5.
Masakazu
On Thu, Sep 1, 2022 at 10:08 AM Walt Karas <wk...@yahooinc.com.invalid>
wrote:
> As a rule of thumb, I prefer using encapsulation/data abstraction. I think
> perhaps that is one reason I've been a poor match to this project. There
> doesn't seem to be a consensus that we should follow this rule of thumb.
>
> KIt, that would be my preference. But I am part of the consensus I think
> we have, that we should favor a series of smaller steps, rather than doing
> all of them in one big step.
>
> On Wed, Aug 31, 2022 at 12:13 PM Shu Kit Chan <ch...@gmail.com>
> wrote:
>
> > Also are we planning to eventually rewrite our existing APIs (where
> > applicable) to use this?
> >
> > On Wed, Aug 31, 2022 at 8:36 AM Masakazu Kitajo <ma...@apache.org>
> wrote:
> > >
> > > What's the advantage of using TSHeapBuf? What issue does it solve?
> > >
> > >
> > > On Wed, Aug 31, 2022 at 7:48 AM Walt Karas <wkaras@yahooinc.com.invalid
> >
> > > wrote:
> > >
> > > > Described here:
> > > >
> > > >
> >
> https://urldefense.com/v3/__https://github.com/apache/trafficserver/blob/os_pkey_cnf_reload/doc/developer-guide/api/functions/TSHeapBuf.en.rst*tsheapbufdata__;Iw!!Op6eflyXZCqGR5I!Du0fBfMhb4pdM2ECFijJ7aJ-jT70jEPeZwjhsvWt2Dr2cSZ5G7HWY20wZOmFHIR3MxnvPZpoRDMlII5dgow$
> >
> > > > ,
> > > >
> > > > In PR
> >
> https://urldefense.com/v3/__https://github.com/apache/trafficserver/pull/8790__;!!Op6eflyXZCqGR5I!Du0fBfMhb4pdM2ECFijJ7aJ-jT70jEPeZwjhsvWt2Dr2cSZ5G7HWY20wZOmFHIR3MxnvPZpoRDMlYh9lIuc$
> > .
> > > >
> > > > This allows a dynamically allocated buffer, of any reasonable length,
> > to be
> > > > returned by a TS API function, like this:
> > > >
> > > > TSHeapBuf hb = TSSomething(x, y, z);
> > > >
> > > > One alternative is an interface like this:
> > > >
> > > > int length;
> > > > char *data = TSSomething(x, y, z, &length);
> > > >
> > > > The data is dynamically allocated, and would be freed with TSfree().
> > > >
> > > > Another alternative is:
> > > >
> > > > char *buf = TSalloc(BUF_SIZE);
> > > > int actual_size = TSSomething(x, y, z, buf, BUF_SIZE);
> > > > if (actual_size > BUF_SIZE) {
> > > > // buf was too small, unchanged.
> > > > TSfree(buf);
> > > > buf = TSalloc(actual_size);
> > > > TSSomething(x, y, z, buf, actual_size);
> > > > }
> > > >
> >
>