You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@trafficserver.apache.org by Leif Hedstrom <zw...@apache.org> on 2023/01/04 22:26:04 UTC
Re: [E] Proposed change to TS API: TSHeapBuf
> 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 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);
> >>>>>>>> }
> >>>>>>>>
> >>>>>>
> >>>>>
> >>>>
> >>
> >>
>
>