| Message ID | 20240324011301.1553072-13-sandals@crustytoothpaste.net (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Support for arbitrary schemes in credentials | expand |
On Sun, Mar 24, 2024 at 01:13:00AM +0000, brian m. carlson wrote: > In a future commit, we'll want the ability to efficiently swap the > contents of two strvec instances without needing to copy any data. > Since a strvec is simply a pointer and two sizes, swapping them is as > simply as copying the two pointers and sizes, so let's do that. > > We use a temporary here simply because C doesn't provide a standard > swapping function, unlike C++ and Rust, but a good optimizing compiler > will recognize this syntax and handle it appropriately using an > optimization pass. > > Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> > --- > strvec.c | 7 +++++++ > strvec.h | 5 +++++ > 2 files changed, 12 insertions(+) > > diff --git a/strvec.c b/strvec.c > index 178f4f3748..93006f1e63 100644 > --- a/strvec.c > +++ b/strvec.c > @@ -106,3 +106,10 @@ const char **strvec_detach(struct strvec *array) > return ret; > } > } > + > +void strvec_swap(struct strvec *a, struct strvec *b) > +{ > + struct strvec t = *a; > + *a = *b; > + *b = t; > +} Isn't this equivalent to `SWAP(*a, *b)`? Patrick
Patrick Steinhardt <ps@pks.im> writes: >> + >> +void strvec_swap(struct strvec *a, struct strvec *b) >> +{ >> + struct strvec t = *a; >> + *a = *b; >> + *b = t; >> +} > > Isn't this equivalent to `SWAP(*a, *b)`? Yes. "make coccicheck" does flag this one. Let's drop this step, and tweak the 13/13 patch to make its sole caller directly use SWAP() instead, perhaps like so: credential.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git c/credential.c w/credential.c index 9a08efe113..5ea52ddd68 100644 --- c/credential.c +++ w/credential.c @@ -39,7 +39,7 @@ void credential_clear(struct credential *c) void credential_next_state(struct credential *c) { strvec_clear(&c->state_headers_to_send); - strvec_swap(&c->state_headers, &c->state_headers_to_send); + SWAP(c->state_headers, c->state_headers_to_send); } void credential_clear_secrets(struct credential *c)
On 2024-03-27 at 21:22:17, Junio C Hamano wrote: > Patrick Steinhardt <ps@pks.im> writes: > > >> + > >> +void strvec_swap(struct strvec *a, struct strvec *b) > >> +{ > >> + struct strvec t = *a; > >> + *a = *b; > >> + *b = t; > >> +} > > > > Isn't this equivalent to `SWAP(*a, *b)`? > > Yes. "make coccicheck" does flag this one. > > Let's drop this step, and tweak the 13/13 patch to make its sole > caller directly use SWAP() instead, perhaps like so: Great, I'll include that in a reroll, likely this weekend.
diff --git a/strvec.c b/strvec.c index 178f4f3748..93006f1e63 100644 --- a/strvec.c +++ b/strvec.c @@ -106,3 +106,10 @@ const char **strvec_detach(struct strvec *array) return ret; } } + +void strvec_swap(struct strvec *a, struct strvec *b) +{ + struct strvec t = *a; + *a = *b; + *b = t; +} diff --git a/strvec.h b/strvec.h index 4715d3e51f..5b762532bb 100644 --- a/strvec.h +++ b/strvec.h @@ -88,4 +88,9 @@ void strvec_clear(struct strvec *); */ const char **strvec_detach(struct strvec *); +/** + * Swap the contents of two `strvec` structs without allocating. + */ +void strvec_swap(struct strvec *, struct strvec *); + #endif /* STRVEC_H */
In a future commit, we'll want the ability to efficiently swap the contents of two strvec instances without needing to copy any data. Since a strvec is simply a pointer and two sizes, swapping them is as simply as copying the two pointers and sizes, so let's do that. We use a temporary here simply because C doesn't provide a standard swapping function, unlike C++ and Rust, but a good optimizing compiler will recognize this syntax and handle it appropriately using an optimization pass. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> --- strvec.c | 7 +++++++ strvec.h | 5 +++++ 2 files changed, 12 insertions(+)