Den fre 20 juni 2025 kl 08:12 skrev Branko Čibej <br...@apache.org>:
> Hi Bert, > > I've been going through the Serf code on trunk, trying to fix compiler > warnings, and found something that I don't understand and don't want to > just "fix" and possibly introduce a bug. I'm asking for your help, as > you're the author of that code: > > > https://svn.apache.org/viewvc/serf/trunk/buckets/hpack_buckets.c?annotate=1926128#l1393 > > > I see a warning that 'status' is assigned but never used, and indeed > that's the case; here is the assignment, on line 1479: > > if (ctx->reuse_item) > { > status = hpack_table_get(ctx->reuse_item, tbl, > &keep_key, NULL, > &keep_val, NULL); > } > > > and the return value is just ignored, even though hpack_table_get() can > return an error. I don't know enough to make a safe decision, but just > removing the assignment and 'status' seems wrong. > > I know it's been a long time since you wrote this. Still, do you have > any suggestions? > > Thanks! > > -- Brane > > Maybe...: keep_key and keep_val are both initialized to NULL. The only use are the next two lines: own_key = (ctx->key && ctx->key != keep_key); own_val = (ctx->val && ctx->val != keep_val); They in turn control if we can reuse the key/value from the ctx or need to copy the string to the entry. hpack_table_get will fail if we send an invalid index (ctx->reuse_item == 0) or if the requested entry isn't found in the table. In both cases it will not modify keep_key and keep_val, remaining null, so own_key/own_val should be false, thus creating a copy. To me it looks like optimization to avoid creating a new copy of the key/value if it already exists in the hpack table. Worst case, if hpack_table_get return an error, a new copy will be made wasting a bit of memory (which would probably be a reasonable thing to do anyway if it fails). Cheers, Daniel
