threads / patch / 60889

patchimap-send: include strbuf.h

Subject: [PATCH 1/1] imap-send: include strbuf.h

## tl;dr

5 messages between Feb 9, 2024 and Feb 11, 2024. Diffs are folded; open one to read it.

replies: 4people: 2as markdown or json

Christian Hesse· Feb 9, 2024, 22:26 UTC · lore
From: Christian Hesse <mail@eworm.de>

We had this fixed in 3307f7dde2ae8f5281d0782f7291a073c9b1cdc2, and it broke again in eea0e59ffbed6e33d171ace5be13cde9faa41639.

We make liberal use of the strbuf API functions and types, but the inclusion of <strbuf.h> comes indirectly by including <http.h>, which does not happen if you build with NO_CURL.

This time make the include conditional... Does that prevent from loosing it again?

Signed-off-by: Christian Hesse <mail@eworm.de>
---
 imap-send.c | 2 ++
 1 file changed, 2 insertions(+)
Show changes to imap-send.c +2 −0
diff --git a/imap-send.c b/imap-send.c
index f2e1947e63..cae494c663 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -34,6 +34,8 @@ typedef void *SSL;
 #endif
 #ifdef USE_CURL_FOR_IMAP_SEND
 #include "http.h"
+#else
+#include "strbuf.h"
 #endif
 
 #if defined(USE_CURL_FOR_IMAP_SEND)

-- 
2.43.1
Junio C Hamano· Feb 9, 2024, 22:42 UTC · re: Christian Hesse · lore

Re: [PATCH 1/1] imap-send: include strbuf.h

Christian Hesse <list@eworm.de> writes:
> From: Christian Hesse <mail@eworm.de>
>
> We had this fixed in 3307f7dde2ae8f5281d0782f7291a073c9b1cdc2,
> and it broke again in eea0e59ffbed6e33d171ace5be13cde9faa41639.
Thanks, already reported and fixed, I believe?
Junio C Hamano· Feb 9, 2024, 22:54 UTC · re: Junio C Hamano · lore

Re: [PATCH 1/1] imap-send: include strbuf.h

Junio C Hamano <gitster@pobox.com> writes:
Show 8 quoted lines
> Christian Hesse <list@eworm.de> writes:
>
>> From: Christian Hesse <mail@eworm.de>
>>
>> We had this fixed in 3307f7dde2ae8f5281d0782f7291a073c9b1cdc2,
>> and it broke again in eea0e59ffbed6e33d171ace5be13cde9faa41639.
>
> Thanks, already reported and fixed, I believe?
Oops, missing link:
https://lore.kernel.org/git/pull.1664.git.git.1706833113569.gitgitgadget@gmail.com/
Christian Hesse· Feb 10, 2024, 20:01 UTC · re: Junio C Hamano · lore

Re: [PATCH 1/1] imap-send: include strbuf.h

Junio C Hamano <gitster@pobox.com> on Fri, 2024/02/09 14:54:
Show 14 quoted lines
> Junio C Hamano <gitster@pobox.com> writes:
> 
> > Christian Hesse <list@eworm.de> writes:
> >  
> >> From: Christian Hesse <mail@eworm.de>
> >>
> >> We had this fixed in 3307f7dde2ae8f5281d0782f7291a073c9b1cdc2,
> >> and it broke again in eea0e59ffbed6e33d171ace5be13cde9faa41639.  
> >
> > Thanks, already reported and fixed, I believe?  
> 
> Oops, missing link:
> 
> https://lore.kernel.org/git/pull.1664.git.git.1706833113569.gitgitgadget@gmail.com/

Sorry, missed that... Probably because the breakage went into 2.43.1, but the upstream fix did not. So sorry for the noise.

Anyway... does it make sense to move the include into the condition?
-- 
main(a){char*c=/*    Schoene Gruesse                         */"B?IJj;MEH"
"CX:;",b;for(a/*    Best regards             my address:    */=0;b=c[a++];)
putchar(b-1/(/*    Chris            cc -ox -xc - && ./x    */b/42*2-3)*42);}
Junio C Hamano· Feb 11, 2024, 02:42 UTC · re: Christian Hesse · lore

Re: [PATCH 1/1] imap-send: include strbuf.h

Christian Hesse <list@eworm.de> writes:
Show 6 quoted lines
>> Oops, missing link:
>> 
>> https://lore.kernel.org/git/pull.1664.git.git.1706833113569.gitgitgadget@gmail.com/
>
> Sorry, missed that... Probably because the breakage went into 2.43.1, but the
> upstream fix did not. So sorry for the noise.

Please don't be. Duplicated reports are much much better than no reports due to "well, this must have been reported already by somebody else". Thanks for reporting.

> Anyway... does it make sense to move the include into the condition?

I do not think so. The original breakage was because it implicitly relied on the fact that http.h, which is included only when USE_CURL_FOR_IMAP_SEND is defined, happens to include strbuf.h, even though the code that does not rely on USE_CURL_FOR_IMAP_SEND do unconditionally rely on the strbuf facility being available to them, possibly combined with the fact that not too many people build imap-send with USE_CURL_FOR_IMAP_SEND disabled.

So the conditional thing still rely on an implicit assumption you are making, i.e. "http.h will forever be including strbuf.h", which is fragile when people from time to time come and make sweeping "header clean-up". Which is a good thing. But we need to be careful, and one way to help us being careful against such a header clean-up is to make sure you include what you use yourself, instead of assuming that somebody else you include will keep doing so.

← back to recent threads