[newRPL] Filesystem related date/time issues

[Claudio L.]

(08-14-2016 11:06 PM)matthiaspaul Wrote:  Hi,

Having found some time to inspect the date/time related code in newRPL's FAT implementation, I added some comments (either helping me to understand and validate the code, or pointing at potential issues to be sorted out). The following snippets also illustrate some fixes and sanity checks I recommend to be included.

In order to keep the general newRPL thread readable, I'm opening a new thread for the topic...

Good idea.

(08-14-2016 11:06 PM)matthiaspaul Wrote:  https://sourceforge.net/p/newrpl/sources..._50g/rtc.c

Thanks for the bogus bcd line, don't even know why it was there. The year 2k problem is a non-issue on the calc, it was sorted out in a chip revision long before the 50g came out in 2006 (EDIT: should read: long before the 49G+ came out in 2003). The RTC assumes the 2 digits are 20xx, unless I misunderstood the Samsung docs.

(08-14-2016 11:06 PM)matthiaspaul Wrote:  

Code:


             tm_mday:5, // day of the month 1-31 (0=day not set)
             // DBG 2016-08-12 MPL: Is there a particular reason why the
             // month entry is defined as a 0-based value, while the FAT dir
             // entry and the RTC register are 1-based? The FAT dir entry
             // allows for special value of 0 for "undefined", something we
             // cannot fully cope with at present

The only reason is POSIX:
http://pubs.opengroup.org/onlinepubs/969...ime.h.html
I just took the struct tm and made it more compact.

(08-14-2016 11:06 PM)matthiaspaul Wrote:  https://sourceforge.net/p/newrpl/sources...datetime.c

Code:


      // DBG 2016-08-12 MPL: Is one loop cycle enough for rollover
      // to be reliably propagated in RTC hardware? I've seen other code actually
      // looping until seconds become > 0 (which can cause delays up to 1 sec, though).

I don't see why more than one loop would be needed. The Samsung docs don't imply anywhere that more than one loop could be needed, this is the only reference to the issue:

Quote:...there is no problem, but, if the value is 0 sec., the year, month, date, hour, and minute may be changed to 2060 (Year), 1 (Month), 1 (Date), 0 (Hour) and 0 (Minute) because of the one second deviation that was mentioned. In this case, the user should re-read from BCDYEAR to BCDSEC if BCDSEC is zero.

(08-14-2016 11:06 PM)matthiaspaul Wrote:  https://sourceforge.net/p/newrpl/sources...ritetime.c

Code:


// CHG 2016-08-13 MPL: Combined FSGetWriteTime(), FSGetCreatTime(), FSGetAccessDate() to avoid redundancy and save code

These are very simple functions that the compiler usually auto-inlines into the calling function. Combining them might prevent auto-inlining from working.
Regarding the sanitization of date/time: the file system does nothing with these values, so invalid dates don't disturb its operation. I think the user code that uses these dates/times is better positioned to decide what to do with an invalid date than the file system layer (which would just mark as invalid and discard everything), so it's better not to touch them.
For example, a program might be looking for all files on a certain year, then doesn't care if the month is invalid as long as the year is good. Other program could be sorting files by date, and needs all fields to be valid. That same program could prefer to list a month 13 after the other files that year (and month 14 would go after 13 and before 15), while if we force it to zero to indicate invalid month, it would have to list it first (and there's no difference between month 13, 14 or 15).
Another advantage of not sanitizing is that the conversion is reversible: if you read an invalid creation date writing it back with the same tm struct would recover the same invalid date, preserving whatever was on disk.