The issue I was thinking about how serf_buckets_are_v2() is/was used.
But checking the current code in serf_get_type() we probably already worked
around this problem for some time...
Bert
---
>From trunk:
const serf_bucket_type_t *serf_get_type(serf_bucket_t *bucket,
int min_version)
{
const serf_bucket_t *r;
switch (min_version) {
case 1:
r = bucket; /* Always supported */
break;
#if 0
case 3:
r = bucket->type->read_bucket(bucket, &v3_check);
break;
#endif
case 2:
#if 0
/* We can't trust this check to always do the right thing as
there
can be multiple pointers to the same function...
but an optimizing compiler/linker can still use the hint if
it
is inlining this function. ### Enable?
*/
if (bucket->type->read_bucket == serf_buckets_are_v2)
return bucket->type;
#endif
r = bucket->type->read_bucket(bucket, &v2_check);
break;
default:
abort();
}
if (r != NULL)
return r->type;
return &fallback_bucket_type;
}
On Tue, Jan 20, 2026 at 11:00 AM Branko Čibej <[email protected]> wrote:
> On 20. 1. 26 09:51, Bert Huijben via dev wrote:
> > Hi All,
> >
> > Not really active here any more. Thanks for picking up where we left
> years
> > ago. Really happy to see that you all move to get things forward again.
> >
> > +1 on what Branko mentioned here... Please be very careful with global
> > state, being used. The rest is nicely per context.
> >
> > --------------
> > One thing I still have on my mind but not written down.... and mostly out
> > of context here:
> >
> > What is not properly documented is that on Windows when you use Serf
> built
> > as DLL the comparison of the functions in one of the Serf macros is not
> > guaranteed to work... And in practice it doesn't work there, so the old
> vs
> > new bucket type detection breaks. I think currently all Windows users
> work
> > around this by using the build static. For the future I think we should
> get
> > that address via a helper function to make sure we can do the
> ==comparison
> > safely.
>
> Are you referring to things like this:
>
> extern const serf_bucket_type_t serf_bucket_type_response;
> #define SERF_BUCKET_IS_RESPONSE(b) SERF_BUCKET_CHECK((b), response)
>
>
> which seems OK to me, though I'd have done this with a function call. Or
> could you point me to the macro you had in mind?
>
> >
> > For 1.3 ignoring those code paths is mostly safe, but for 1.5+ it should
> be
> > made safe or the limitations documented
>
> Agreed.
>
>
> -- Brane
>
