Re: [chrony-dev] [PATCH v4 1/3] sys_linux: report if CAP_SYS_TIME is not present

[Miroslav Lichvar <mlichvar@xxxxxxxxxx>]

On Wed, Mar 14, 2018 at 03:05:29PM +0100, Christian Ehrhardt wrote:
> Instead of having adjtimex just fail with a permission issue
> improve the error messaging by warning for the lack of
> CAP_SYS_TIME on SYS_Linux_Initialise.
> 
> Message will look like (instead of only the latter message):
>  CAP_SYS_TIME not present
>  adjtimex(0x8001) failed : Operation not permitted

Looks good.

>  /* ================================================== */
> +
> +static void SYS_Linux_ReportTimeAdjustBlockers(void)

Following the coding style, "static void" should be on its own line.
Also, as a static function it should be named report_time_adjust_blockers.

> +{
> +  if (CAP_IS_SUPPORTED(CAP_SYS_TIME) && cap_get_bound(CAP_SYS_TIME))
> +    return;
> +  LOG(LOGS_WARN, "CAP_SYS_TIME not present");

This code needs to be wrapped in #ifdef FEAT_PRIVDROP, otherwise it
will cause the compilation to fail without libcap even if no other
code is using the function.

> +}
> +
> +/* ================================================== */
>  /* Initialisation code for this module */
>  
>  void
> @@ -388,6 +397,10 @@ SYS_Linux_Initialise(void)
>  {
>    get_version_specific_details();
>  
> +#if defined (FEAT_PRIVDROP)
> +  SYS_Linux_ReportTimeAdjustBlockers();
> +#endif

The #if here is unnecessary.

I can fix these and push it if it's ok with you.

Thanks,

-- 
Miroslav Lichvar

-- 
To unsubscribe email chrony-dev-request@xxxxxxxxxxxxxxxxxxxx with "unsubscribe" in the subject.
For help email chrony-dev-request@xxxxxxxxxxxxxxxxxxxx with "help" in the subject.
Trouble?  Email listmaster@xxxxxxxxxxxxxxxxxxxx.