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 <mas...@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 <wka...@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 <mas...@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 <wka...@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 <mas...@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
> > <wka...@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 <
> > chanshu...@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 <
> > mas...@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
> > > > > <wka...@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);
> > > > > > > > > }
> > > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
