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); > > > > } > > > > > > >
