On Mon, Nov 29, 2021 at 12:51 AM kuroda.hay...@fujitsu.com < kuroda.hay...@fujitsu.com> wrote:
> Dear Zhihong, > > Thank you for giving comments! I'll post new patches later. > > > +#define HOLD_CHECKING_REMOTE_SERVERS_INTERRUPTS() > (CheckingRemoteServersHoldoffCount++) > > > > The macro contains only one operation. Can the macro be removed (with > `CheckingRemoteServersHoldoffCount++` inlined) ? > > Hmm, these HOLD/RESUME macros are followed HOLD_INTERRUPUST() and > HOLD_CANCEL_INTERRUPTS(): > > ``` > #define HOLD_INTERRUPTS() (InterruptHoldoffCount++) > > #define RESUME_INTERRUPTS() \ > do { \ > Assert(InterruptHoldoffCount > 0); \ > InterruptHoldoffCount--; \ > } while(0) > > #define HOLD_CANCEL_INTERRUPTS() (QueryCancelHoldoffCount++) > > #define RESUME_CANCEL_INTERRUPTS() \ > do { \ > Assert(QueryCancelHoldoffCount > 0); \ > QueryCancelHoldoffCount--; \ > } while(0) > > #define START_CRIT_SECTION() (CritSectionCount++) > > #define END_CRIT_SECTION() \ > do { \ > Assert(CritSectionCount > 0); \ > CritSectionCount--; \ > } while(0) > ``` > > So I want to keep the current style. Could you tell me if you have any > other reasons? > > > + if (CheckingRemoteServersTimeoutPending && > CheckingRemoteServersHoldoffCount != 0) > > + { > > + /* > > + * Skip checking foreign servers while reading messages. > > + */ > > + InterruptPending = true; > > + } > > + else if (CheckingRemoteServersTimeoutPending) > > > > Would the code be more readable if the above two if blocks be moved > inside one enclosing if block (factoring the common condition)? > > > > + if (CheckingRemoteServersTimeoutPending) > > +1. Will fix. > > Best Regards, > Hayato Kuroda > FUJITSU LIMITED > Hi, It is Okay to keep the macros. Thanks