<div dir="ltr"><div class="gmail_default" style="font-family:monospace,monospace"><br></div><div class="gmail_extra">Cyril Hrubis <span dir="ltr"><<a href="mailto:chrubis@suse.cz" target="_blank">chrubis@suse.cz</a>></span> wrote:<br><span class="gmail-m_-8514980772807819230gmail-"></span><br><span class="gmail-m_-8514980772807819230gmail-"></span><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="gmail-m_-8514980772807819230gmail-">
> +#define TST_RETRY_FUNC_WITH_EXPONENTIA<wbr>L_BACKOFF(FUNC, ERET, MAX_DELAY) \<br>
<br>
</span>That is quite a long name, I guess that we can shorten it a bit without<br>
loosing too much clarity. Something as TST_RETRY_FN_EXP_BACKOFF()<br>
carries all the information and is a bit shorter.<br></blockquote><div><div style="display:inline" class="gmail_default"><span style="font-family:arial,helvetica,sans-serif"><br></span></div><div style="display:inline" class="gmail_default"><span style="font-family:arial,helvetica,sans-serif">Yes, and we can also take use this directly for looping more seconds.<br><br></span></div><span style="font-family:arial,helvetica,sans-serif"></span></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<span class="gmail-m_-8514980772807819230gmail-"><br>
> +do { int delay = 1; \<br>
<br>
</span>We should at least prefix the delay with tst_ to make sure that it will<br>
not alias with anything that has been passed to the FUNC, e.g. if the<br>
FUNC is defined as foo_func(delay); the delay variable will be aliased<br>
and the function will do something very unexpected.<br></blockquote><div><div class="gmail_default"><span style="font-family:arial,helvetica,sans-serif"><br></span></div><div class="gmail_default"><span style="font-family:arial,helvetica,sans-serif">You are right, I missed this problem. <br></span></div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<span class="gmail-m_-8514980772807819230gmail-"><br>
> + for (;;) { \<br>
> + typeof(FUNC) ret = FUNC; \<br>
> + if (ret == (typeof(FUNC))ERET) \<br>
> + break; \<br>
<br>
</span>Do we really need the (typeof(FUNC)) cast here?<br></blockquote><div class="gmail_default"><span style="font-family:arial,helvetica,sans-serif"><br>I set the type cast to ensure more safe in comparing, but it seems no needed.<br></span></div><div class="gmail_default"><span style="font-family:arial,helvetica,sans-serif">Will remove this.<br><br></span></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<span class="gmail-m_-8514980772807819230gmail-"><br>
> + if (delay < MAX_DELAY * 1000000) { \<br>
> + usleep(delay); \<br>
> + delay *= 2; \<br>
<br>
</span>Maybe we can be a bit more verbose and say something as:<br>
<br>
tst_res(TINFO, #FUNC" returned %i, retrying in %ius", ret, delay);<br></blockquote><div><div style="display:inline" class="gmail_default"><span style="font-family:arial,helvetica,sans-serif"><br>Agree.<br><br></span></div><span style="font-family:arial,helvetica,sans-serif"></span></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<span class="gmail-m_-8514980772807819230gmail-"><br>
> + } else { \<br>
> + tst_brk_(__FILE__, __LINE__, \<br>
> + TBROK | TERRNO, #FUNC " failed"); \<br>
<br>
</span>As far as I can tell we can just use the plain tst_brk() in macros since<br>
the __LINE__ will be a constant in the whole macro and will represent<br>
the line where the macro is called.<br>
<br>
Also I'm not sure that adding the TERRNO here is a good idea, I suppose<br>
that there may be a retry functions that are not seting it on a failure.<br>
Maybe we can pass additional tst_res/tst_brk flags to the macro itself.<br>
<span class="gmail-m_-8514980772807819230gmail-"><br></span></blockquote><div><br><div style="font-family:arial,helvetica,sans-serif;display:inline" class="gmail_default">Hmm, yes! But I don't want to add more flags to the macro since that will<br></div><div style="font-family:arial,helvetica,sans-serif;display:inline" class="gmail_default">make things more complicated for user. So, let's just abandon the TERRNO<br>to see how thing going on.<br></div><div style="font-family:arial,helvetica,sans-serif;display:inline" class="gmail_default"></div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="gmail-m_-8514980772807819230gmail-">
> + } \<br>
> + } \<br>
> +} while(0)<br>
> +<br>
> +/*<br>
> + * Exponential backoff usleep for wating a varible change<br>
> + * @VAR: the variable will be changed in other place<br>
> + * @EXP: an expected value which VAR should be equal to<br>
> + */<br>
> +#define TST_WAIT_ON_VAR(VAR, EXP) \<br>
> + TST_WAIT_WITH_EXPONENTIAL_BAC<wbr>KOFF(VAR, EXP, 1)<br>
> +<br>
> +#define TST_WAIT_WITH_EXPONENTIAL_BACK<wbr>OFF(VAR, EXP, MAX_DELAY) \<br>
> +do { int delay = 1; \<br>
</span> ^<br>
Trailing whitespace.<br>
<span class="gmail-m_-8514980772807819230gmail-">> + for (;;) { \<br>
> + if (VAR == (typeof(VAR)) EXP) \<br>
> + break; \<br>
> + if (delay < MAX_DELAY * 1000000) { \<br>
> + usleep(delay); \<br>
> + delay *= 2; \<br>
> + } else { \<br>
> + tst_brk_(__FILE__, __LINE__, \<br>
> + TBROK | TERRNO, #VAR " is not expected"); \<br>
> + } \<br>
> + } \<br>
> +} while(0)<br>
<br>
</span>I would refrain from adding this function unless we have a use-case<br>
already. Do you have a test in mind that could use this?<br></blockquote><div><span style="font-family:arial,helvetica,sans-serif"><br>No, I don't have any test so far, this is just came to my mind</span><br><span style="font-family:arial,helvetica,sans-serif"></span><span style="font-family:arial,helvetica,sans-serif"></span><div style="display:inline" class="gmail_default"><span style="font-family:arial,helvetica,sans-serif">for one situation. We could re-add it if necessary next time.</span></div><span style="font-family:arial,helvetica,sans-serif"> </span><br><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div class="gmail-m_-8514980772807819230gmail-HOEnZb"><div class="gmail-m_-8514980772807819230gmail-h5"><br>
> #endif /* TST_COMMON_H__ */<br>
> --<br>
> 1.9.3<br>
><br>
<br>
--<br>
</div></div><span class="gmail-m_-8514980772807819230gmail-HOEnZb"><font color="#888888">Cyril Hrubis<br>
<a href="mailto:chrubis@suse.cz" target="_blank">chrubis@suse.cz</a><br>
</font></span></blockquote></div><br><br clear="all"><br>-- <br><div class="gmail-m_-8514980772807819230gmail_signature">Li Wang<br><a href="mailto:liwang@redhat.com" target="_blank">liwang@redhat.com</a></div>
</div></div>