[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-hotspot-runtime-dev
Subject:    RE: RFR(S): 8154996: [aix] Implement compare_file_modified_times for "8146879: Add option ..."
From:       "Lindenmaier, Goetz" <goetz.lindenmaier () sap ! com>
Date:       2016-04-25 16:43:00
Message-ID: 2c321aa9e139428cb775a77dec2e8169 () DEWDFE13DE09 ! global ! corp ! sap
[Download RAW message or body]

Hi Volker, 

I also wondered about the semantics of the method. 
I think it's just supposed to return negative or positive
differences to check which is the newer file.
Stat contains different time representations on the platforms. 
GNU returns a struct with seconds and nanoseconds, therefore
they check both. On AIX, a single value time_t is returned. 
(If you compile with _GNU_SOURCE set, AIX 7.1 knows the
other format.).  But all that should have been mentioned
in the review of 8146879.

Thanks for the review of my change, 
  Goetz.

> -----Original Message-----
> From: Volker Simonis [mailto:volker.simonis@gmail.com]
> Sent: Monday, April 25, 2016 5:29 PM
> To: Lindenmaier, Goetz <goetz.lindenmaier@sap.com>
> Cc: hotspot-runtime-dev@openjdk.java.net
> Subject: Re: RFR(S): 8154996: [aix] Implement compare_file_modified_times
> for "8146879: Add option ..."
> 
> Hi Goetz,
> 
> your change looks good.
> 
> I'm only a little confused about the original change for 8146879 where
> 'compare_file_modified_times()' returns seconds on some platforms and
> seconds or nanoseconds on others. It's also not clear if "better than
> second" resolution is required (or desired). If that would improve the
> functionality we could additionally use the st_mtime_n field on AIX
> which provides nanosecond resolution. But as your change is "as good
> as" Solaris, I think its OK to push it and do the possible enhancement
> in a follow up change.
> 
> Regards,
> Volker
> 
> 
> On Mon, Apr 25, 2016 at 1:03 PM, Lindenmaier, Goetz
> <goetz.lindenmaier@sap.com> wrote:
> > Hi
> >
> > I need another fix for AIX to 8146879. This is AIX-only, though:
> > http://cr.openjdk.java.net/~goetz/wr16/8154996-
> aix_compFile/webrev.01/
> >
> > It simply implements the new OS function.  Please review.
> >
> > Thanks,
> > Goetz.

[prev in list] [next in list] [prev in thread] [next in thread]