On Fri, Oct 31, 2025 at 05:47:51AM +0100, Theo Buehler wrote:
> On Thu, Oct 30, 2025 at 11:24:56PM +0000, Job Snijders wrote:
> > On Thu, Oct 30, 2025 at 09:39:01PM +0100, Theo Buehler wrote:
> > > The output.c changes except the one in outputheader() are largely
> > > unrelated and should be done separately. The rest is ok with me.
> > 
> > ack - below is just the unrelated part
> > 
> > With this diff the utility sets the "modification time" of the output
> > file to be the buildtime timestamp.
> > 
> > To me this is quite useful in analytics pipelines: moving & archiving
> > files using the timestamp from the filesystem instead of having to
> > deserialize the file content to find the timestamp shaves off a few
> > seconds here and there.
> > 
> > OK?
> 
> Yes, it's all good, I understand. It's just that futimensat() is a
> strange and confusing interface with its partially initialized array and
> all those magic values... I am also wondering if this dance wants
> factoring into a separate helper shared between output.c and repo.c, but
> that might obfuscate more than it helps.

Why not use futimes(2) or futimens(2) before the close of the fd?
In either case you can set both times to the same value. It is ok to set
the access time to the same value.
 
> ok tb
> 
> > 
> > Index: output.c
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/rpki-client/output.c,v
> > diff -u -p -r1.43 output.c
> > --- output.c	30 Oct 2025 23:18:06 -0000	1.43
> > +++ output.c	30 Oct 2025 23:20:08 -0000
> > @@ -76,7 +76,7 @@ static const struct outputs {
> >  
> >  static FILE	*output_createtmp(char *);
> >  static void	 output_cleantmp(void);
> > -static int	 output_finish(FILE *);
> > +static int	 output_finish(FILE *, time_t);
> >  static void	 sig_handler(int);
> >  static void	 set_signal_handler(void);
> >  
> > @@ -152,7 +152,7 @@ outputfiles(struct validation_data *vd, 
> >  			rc = 1;
> >  			continue;
> >  		}
> > -		if (output_finish(fout) != 0) {
> > +		if (output_finish(fout, vd->buildtime) != 0) {
> >  			warn("finish for %s format failed", outputs[i].name);
> >  			output_cleantmp();
> >  			rc = 1;
> > @@ -187,12 +187,23 @@ output_createtmp(char *name)
> >  }
> >  
> >  static int
> > -output_finish(FILE *out)
> > +output_finish(FILE *out, time_t buildtime)
> >  {
> > +	struct timespec ts[2];
> > +
> >  	if (fclose(out) != 0)
> >  		return -1;
> > +
> > +	ts[0].tv_nsec = UTIME_OMIT;
> > +	ts[1].tv_sec = buildtime;
> > +	ts[1].tv_nsec = 0;
> > +
> > +	if (utimensat(AT_FDCWD, output_tmpname, ts, 0) == -1)
> > +		return -1;
> > +
> >  	if (rename(output_tmpname, output_name) == -1)
> >  		return -1;
> > +
> >  	output_tmpname[0] = '\0';
> >  	return 0;
> >  }
> 

-- 
:wq Claudio