[Piglit] [PATCH 2/2] piglit-util: add piglit_get_microseconds

[Chad Versace]

On 10/17/2013 02:22 PM, Jordan Justen wrote:
> On Thu, 2013-10-17 at 13:56 -0700, Chad Versace wrote:
>> On 10/17/2013 10:52 AM, Jordan Justen wrote:
>>> v2:
>>>    * Use Chad's cmake detection
>>>    * return value is signed, and a negative indicates an error
>>>
>>> Signed-off-by: Jordan Justen <jordan.l.justen at intel.com>
>>> Cc: Chad Versace <chad.versace at linux.intel.com>
>>> Cc: Ian Romanick <idr at freedesktop.org>
>>> ---
>>>    tests/util/piglit-util.c |   20 ++++++++++++++++++++
>>>    tests/util/piglit-util.h |   10 ++++++++++
>>>    2 files changed, 30 insertions(+)
>>>
>>> diff --git a/tests/util/piglit-util.c b/tests/util/piglit-util.c
>>> index 71d55a7..8836978 100644
>>> --- a/tests/util/piglit-util.c
>>> +++ b/tests/util/piglit-util.c
>>> @@ -33,6 +33,10 @@
>>>    #include <string.h>
>>>    #include <errno.h>
>>>
>>> +#ifdef PIGLIT_HAS_POSIX_CLOCK_MONOTONIC
>>> +#include <time.h>
>>> +#endif
>>> +
>>>    #include "config.h"
>>>    #if defined(HAVE_SYS_TIME_H) && defined(HAVE_SYS_RESOURCE_H) && defined(HAVE_SETRLIMIT)
>>>    #include <sys/time.h>
>>> @@ -462,3 +466,19 @@ write_null:
>>>    	va_end(va);
>>>    	return size_written;
>>>    }
>>> +
>>> +int64_t
>>> +piglit_get_microseconds(void)
>>> +{
>>> +#ifdef PIGLIT_HAS_POSIX_CLOCK_MONOTONIC
>>> +	struct timespec t;
>>> +	int r = clock_gettime(CLOCK_MONOTONIC, &t);
>>> +	if (r >= 0)
>>> +		return (t.tv_sec * 1000000) + (t.tv_nsec / 1000);
>>> +	else
>>> +		return -1LL;
>>> +#else
>>> +	return (int64_t) -1;
>>> +#endif
>>> +}
>>
>> The two occurrences of `return -1` should appear symmetric, otherwise it
>> looks like the code is doing different things at each occurrence.
>>
>> I don't think there's any need for the int64_t cast, but I'm not confident
>> of that. Do you get a warning if you remove that?
>
> Whoops. Yes, both should just use the non-cast version.
>
>> Anways, please make the returns look symmetric and this has my r-b. I think
>> piglit_get_microseconds() is much less cumbersome to use than the Posix
>> clock functions.
>
> Are you okay with my changes to your patch 1 as well?

Yes, patch 1 looks good too and gets an r-b.