Paolo Bonzini wrote: > On 09/16/2011 01:01 PM, Jim Meyering wrote: >> + >> +#if ! MBS_SUPPORT >> +# undef MB_CUR_MAX >> +# define MB_CUR_MAX 1 >> +#endif > > Thanks for splitting the tail of the series. I'm still a bit nervous > about redefining a libc macro. > > What about changing this patch to a sweeping s/MB_CUR_MAX/GREP_MB_MAX/g > > and doing > > #define GREP_MB_MAX (MBS_SUPPORT ? MB_CUR_MAX : 1) > > instead?
I see no reason to worry about such a redefinition. A quick look through glibc sources shows that it is only defined in .h files, never used. All of the uses in grep/gnulib look fine. Any code that uses MB_CUR_MAX when MBS_SUPPORT is 0 should be prepared to deal with a "1". Besides, introducing a new symbol name like that would make the code slightly less readable. If a problem arises, and that's the best solution, I'll be happy to do it, but I would create/use a new name like that only as a last resort. > It should be easy to do a global search-and-replace on the patch files > so that they apply on top of a tree that uses GREP_MB_MAX. > > Also, I am not sure why patch 2/5 is there if it fixes a compilation > failure, and not at the beginning to prevent the failure? Indeed, it does not depend on 1/5, so I've pushed it first. Thanks again for the review.
