> On 23 Apr 2025, at 02:01, Jacob Champion
> wrote:
> On Tue, Apr 22, 2025 at 3:10 AM Daniel Gustafsson wrote:
>> The attached
>> v3 allocates via the JSON api, no specific error handling should be required
>> as
>> it's already handled today.
>
> pgindent shows one whitespace change on my ma
On Tue, Apr 22, 2025 at 3:10 AM Daniel Gustafsson wrote:
> My preference is that no operation can silently work on a failed object, but
> it's not a hill (even more so given where we are in the cycle).
Hm, okay. Something to talk about with Andrew and Peter, maybe.
> The attached
> v3 allocates
> On 21 Apr 2025, at 20:58, Jacob Champion
> wrote:
> Personally, I'm fine with can't-fail APIs, as long as they're
> documented as such. (I think the deferred error API was probably
> chosen so that src/common JSON clients could be written without a lot
> of pain?)
My preference is that no ope
On Mon, Apr 21, 2025 at 11:46 AM Daniel Gustafsson wrote:
> We do, but with the current coding we call setJsonLexContextOwnsTokens
> immediately after creation which derefences the pointer without checkinf for
> allocation failure.
Right. (The flag does nothing on the OOM sentinel.)
> This means
> On 21 Apr 2025, at 20:28, Jacob Champion
> wrote:
>
> On Mon, Apr 21, 2025 at 11:20 AM Daniel Gustafsson wrote:
>> Sure, but I fear we'll get an endless stream of static analysis reports for
>> the
>> allocation leaking if we don't free it.
>
> But we do free it, in freeJsonLexContext(). Th
On Mon, Apr 21, 2025 at 11:20 AM Daniel Gustafsson wrote:
> Sure, but I fear we'll get an endless stream of static analysis reports for
> the
> allocation leaking if we don't free it.
But we do free it, in freeJsonLexContext(). That usage of the API goes
back to 2023, with 1c99cde2f344. Or am I
> On 21 Apr 2025, at 17:33, Jacob Champion
> wrote:
>
> On Sat, Apr 19, 2025 at 2:15 PM Daniel Gustafsson wrote:
>> Since there is no way to determine if the allocation succeeded from outside
>> of
>> the JSON api it might be better to keep the calloc and explicitly free it?
>
> I don't think
On Sat, Apr 19, 2025 at 2:15 PM Daniel Gustafsson wrote:
> Since there is no way to determine if the allocation succeeded from outside of
> the JSON api it might be better to keep the calloc and explicitly free it?
I don't think so; pg_parse_json() will error out quickly, so I don't
see much adva
> On 17 Apr 2025, at 17:48, Jacob Champion
> wrote:
>
> On Thu, Apr 17, 2025 at 8:20 AM Tom Lane wrote:
>> I confirm this silences those warnings on my Fedora 41 box.
>
> Instead of doing
>
>lex = calloc(...);
>/* (error out on NULL return) */
>makeJsonLexContextCstringLen(lex, ..
Jacob Champion writes:
> On Wed, Apr 16, 2025 at 4:04 PM Tom Lane wrote:
>> Looking through all of the callers of freeJsonLexContext, quite
>> a lot of them use local JsonLexContext structs, and probably some
>> of them are more performance-critical than these. So that raises
>> the question of
On Thu, Apr 17, 2025 at 8:20 AM Tom Lane wrote:
> I confirm this silences those warnings on my Fedora 41 box.
Instead of doing
lex = calloc(...);
/* (error out on NULL return) */
makeJsonLexContextCstringLen(lex, ...);
we need to do
lex = makeJsonLexContextCstringLen(NULL, ...)
Daniel Gustafsson writes:
> Agreed, moving to heap allocated structures for these callsites seem much
> better. Something like the attached should be enough I think?
I confirm this silences those warnings on my Fedora 41 box.
I'm content to do it like this, but maybe Jacob wants to
investigate a
> On 17 Apr 2025, at 01:28, Tom Lane wrote:
>
> Jacob Champion writes:
>> On Wed, Apr 16, 2025 at 4:04 PM Tom Lane wrote:
>>> Looking through all of the callers of freeJsonLexContext, quite
>>> a lot of them use local JsonLexContext structs, and probably some
>>> of them are more performance-cr
Daniel Gustafsson writes:
>> On 16 Apr 2025, at 23:42, Tom Lane wrote:
>> I'm not sure
>> how other than giving up on stack allocation of JsonLexContexts,
>> though, especially if we consider the jsonapi API frozen. But seeing
>> that there are only three such call sites and none of them seem in
Daniel Gustafsson writes:
> On 17 Apr 2025, at 00:12, Tom Lane wrote:
>> The only alternative I can see that might stop the warning is if we
>> can find a way to make it clearer to the optimizer that the FREE()
>> isn't reached. But I'm not sure about a trustworthy way to make that
>> happen.
>
> On 16 Apr 2025, at 23:42, Tom Lane wrote:
> It seems fairly dangerous to ignore -Wfree-nonheap-object warnings.
> I feel like we ought to move to prevent these somehow.
Absolutely agree.
> I'm not sure
> how other than giving up on stack allocation of JsonLexContexts,
> though, especially if
On Wed, Apr 16, 2025 at 4:04 PM Tom Lane wrote:
> Looking through all of the callers of freeJsonLexContext, quite
> a lot of them use local JsonLexContext structs, and probably some
> of them are more performance-critical than these. So that raises
> the question of why are we seeing warnings for
> On 17 Apr 2025, at 00:12, Tom Lane wrote:
>
> Daniel Gustafsson writes:
>>> On 16 Apr 2025, at 23:42, Tom Lane wrote:
>>> I'm not sure
>>> how other than giving up on stack allocation of JsonLexContexts,
>>> though, especially if we consider the jsonapi API frozen. But seeing
>>> that there
Em qua., 16 de abr. de 2025 às 18:42, Tom Lane escreveu:
> I noticed some new warnings from buildfarm member chafer,
> which I'm able to reproduce locally on a Fedora 41 box
> by building with "meson setup build -Db_lto=true":
>
> ninja: Entering directory `build'
> [1515/2472] Linking target src
I noticed some new warnings from buildfarm member chafer,
which I'm able to reproduce locally on a Fedora 41 box
by building with "meson setup build -Db_lto=true":
ninja: Entering directory `build'
[1515/2472] Linking target src/interfaces/libpq/libpq.so.5.18
In function 'freeJsonLexContext',
20 matches
Mail list logo
