Re: [PATCH] for_each_string_list_item(): behave correctly for empty list

2017-09-20 Thread Kaartic Sivaraam
Hi, Though this thread seems to have reached a conclusion, I just wanted to know what I was missing about the optimisation. On Wednesday 20 September 2017 08:00 AM, Jonathan Nieder wrote: From that link: for ( ;valid_int && *valid_int < 10; (*valid_int)++) { printf("Valid

Re: [PATCH] for_each_string_list_item(): behave correctly for empty list

2017-09-19 Thread Junio C Hamano
Jonathan Nieder writes: > I'll send a patch with a commit message saying so to try to close out > this discussion. Thanks. One less thing we have to worry about ;-)

Re: [PATCH] for_each_string_list_item(): behave correctly for empty list

2017-09-19 Thread Junio C Hamano
Jonathan Nieder writes: > ... But a quick test with gcc 4.8.4 > -O2 finds that at least this compiler does not contain such an > optimization. The overhead Michael Haggerty mentioned is real. Still, I have a feeling that users of string_list wouldn't care the overhead of

Re: [PATCH] for_each_string_list_item(): behave correctly for empty list

2017-09-19 Thread Jonathan Nieder
Hi, Kaartic Sivaraam wrote: > On Saturday 16 September 2017 09:36 AM, Michael Haggerty wrote: >> Jonathan Nieder wrote: >>> Does the following alternate fix work? I think I prefer it because >>> it doesn't require introducing a new global. [...] >>> #define

Re: [PATCH] for_each_string_list_item(): behave correctly for empty list

2017-09-19 Thread Jonathan Nieder
Hi, Junio C Hamano wrote: > Kaartic Sivaraam writes: >> On Saturday 16 September 2017 09:36 AM, Michael Haggerty wrote: >>> Jonathan Nieder wrote: Does the following alternate fix work? I think I prefer it because it doesn't require introducing a new

Re: [PATCH] for_each_string_list_item(): behave correctly for empty list

2017-09-19 Thread Junio C Hamano
Kaartic Sivaraam writes: > On Saturday 16 September 2017 09:36 AM, Michael Haggerty wrote: >>> Does the following alternate fix work? I think I prefer it because >>> it doesn't require introducing a new global. [...] >>> #define

Re: [PATCH] for_each_string_list_item(): behave correctly for empty list

2017-09-19 Thread Kaartic Sivaraam
On Saturday 16 September 2017 09:36 AM, Michael Haggerty wrote: Does the following alternate fix work? I think I prefer it because it doesn't require introducing a new global. [...] #define for_each_string_list_item(item,list) \ - for (item = (list)->items; item < (list)->items +

Re: [PATCH] for_each_string_list_item(): behave correctly for empty list

2017-09-19 Thread SZEDER Gábor
On Tue, Sep 19, 2017 at 3:38 PM, SZEDER Gábor wrote: > A bit "futuristic" option along these lines could be something like > this, using a scoped loop variable in the outer loop to ensure that > it's executed at most once: > > #define for_each_string_list_item(item,list) \

Re: [PATCH] for_each_string_list_item(): behave correctly for empty list

2017-09-19 Thread SZEDER Gábor
On Tue, Sep 19, 2017 at 8:51 AM, Michael Haggerty wrote: > On 09/19/2017 02:08 AM, Stefan Beller wrote: >>> I am hoping that this last one is not allowed and we can use the >>> "same condition is checked every time we loop" version that hides >>> the uglyness inside the

Re: [PATCH] for_each_string_list_item(): behave correctly for empty list

2017-09-19 Thread Michael Haggerty
On 09/19/2017 02:08 AM, Stefan Beller wrote: >> I am hoping that this last one is not allowed and we can use the >> "same condition is checked every time we loop" version that hides >> the uglyness inside the macro. > > By which you are referring to Jonathans solution posted. > Maybe we can

Re: [PATCH] for_each_string_list_item(): behave correctly for empty list

2017-09-18 Thread Stefan Beller
> I am hoping that this last one is not allowed and we can use the > "same condition is checked every time we loop" version that hides > the uglyness inside the macro. By which you are referring to Jonathans solution posted. Maybe we can combine the two solutions (checking for thelist to not be

Re: [PATCH] for_each_string_list_item(): behave correctly for empty list

2017-09-17 Thread Junio C Hamano
Michael Haggerty writes: > *sigh* of course you're right. I should know better than to "fire off a > quick fix to the mailing list". > > I guess the two proposals that are still in the running for rescuing > this macro are Jonathan's and Gábor's. I have no strong preference

Re: [PATCH] for_each_string_list_item(): behave correctly for empty list

2017-09-17 Thread Michael Haggerty
On 09/17/2017 02:59 AM, Junio C Hamano wrote: > Michael Haggerty writes: > >> If you pass a newly-initialized or newly-cleared `string_list` to >> `for_each_string_list_item()`, then the latter does >> >> for ( >> item = (list)->items; /* note, this is NULL

Re: [PATCH] for_each_string_list_item(): behave correctly for empty list

2017-09-17 Thread Michael Haggerty
On 09/16/2017 01:51 PM, SZEDER Gábor wrote: It would be a pain to have to change the signature of this macro, and we'd prefer not to add overhead to each iteration of the loop. So instead, whenever `list->items` is NULL, initialize `item` to point at a dummy `string_list_item`

Re: [PATCH] for_each_string_list_item(): behave correctly for empty list

2017-09-16 Thread Junio C Hamano
Michael Haggerty writes: > If you pass a newly-initialized or newly-cleared `string_list` to > `for_each_string_list_item()`, then the latter does > > for ( > item = (list)->items; /* note, this is NULL */ > item < (list)->items + (list)->nr; /*

Re: [PATCH] for_each_string_list_item(): behave correctly for empty list

2017-09-16 Thread SZEDER Gábor
> >> It would be a pain to have to change the signature of this macro, and > >> we'd prefer not to add overhead to each iteration of the loop. So > >> instead, whenever `list->items` is NULL, initialize `item` to point at > >> a dummy `string_list_item` created for the purpose. > > > > What

Re: [PATCH] for_each_string_list_item(): behave correctly for empty list

2017-09-15 Thread Michael Haggerty
On 09/15/2017 08:43 PM, Jonathan Nieder wrote: > Michael Haggerty wrote: > >> If you pass a newly-initialized or newly-cleared `string_list` to >> `for_each_string_list_item()`, then the latter does >> >> for ( >> item = (list)->items; /* note, this is NULL */ >> item

Re: [PATCH] for_each_string_list_item(): behave correctly for empty list

2017-09-15 Thread Jonathan Nieder
Hi, Michael Haggerty wrote: > If you pass a newly-initialized or newly-cleared `string_list` to > `for_each_string_list_item()`, then the latter does > > for ( > item = (list)->items; /* note, this is NULL */ > item < (list)->items + (list)->nr; /* note: NULL + 0 */ >

[PATCH] for_each_string_list_item(): behave correctly for empty list

2017-09-15 Thread Michael Haggerty
If you pass a newly-initialized or newly-cleared `string_list` to `for_each_string_list_item()`, then the latter does for ( item = (list)->items; /* note, this is NULL */ item < (list)->items + (list)->nr; /* note: NULL + 0 */ ++item) Even though this