Hi Federico,

Thanks for the explanation. It has been apparently a long time since I
worked on the internal details of PdfDate. In my opinion, comparing to
time_t(-1) makes sense for invalid dates.
I will update the test accordingly.

Best regards,
 Dominik

On Sat, Jan 9, 2021 at 12:56 AM Federico Kircheis <
federico.kirch...@gmail.com> wrote:

> On 07/01/2021 11.15, Dominik Seichter wrote:
> > Hi Federico,
> >
> > Updated set of patches is pushed with an additional test.
> >
> > One weird issue:
> >
> > void DateTest::testParseDateInvalid()
> > {
> >      PdfString tmp("D:2012020");
> >      PdfDate date(tmp);
> >
> >      struct tm  _tm;
> >      memset (&_tm, 0, sizeof(struct tm));
> >
> >      const time_t t = date.GetTime();
> >      localtime_r(&t, &_tm);
> >
> >      CPPUNIT_ASSERT_EQUAL(false, date.IsValid());
> >      CPPUNIT_ASSERT_EQUAL_MESSAGE("Year", 1970, _tm.tm_year + 1900);
> >      CPPUNIT_ASSERT_EQUAL_MESSAGE("Month", 1, _tm.tm_mon + 1);
> >      CPPUNIT_ASSERT_EQUAL_MESSAGE("Day", 1, _tm.tm_mday);
> >      CPPUNIT_ASSERT_EQUAL_MESSAGE("Hour", 0, _tm.tm_hour);
> >      CPPUNIT_ASSERT_EQUAL_MESSAGE("Minute", 59, _tm.tm_min); // Why 59
> > and not 0?
> >      CPPUNIT_ASSERT_EQUAL_MESSAGE("Second", 59, _tm.tm_sec); // Why 59
> > and not 0?
> > }
> >
> > Any quick ideas why we get 59 here and not 0?
> >
> >
> > Best regards,
> >   Dominik
> >
>
> Sorry, missed your mail.
>
> `time_t(0)` is 1970, so time_t(-1) is one second before, the values make
> sense, even if -1 is "often" (as in PDFDate) used to denote an invalid
> value.
>
> https://en.cppreference.com/w/c/chrono/time_t
>
>
> time_t(-1) is set in the constructor before parsing, and the value is
> updated to something else only on successful parsing.
> It was already the case before my patches.
>
> But does it make sense to test `date.GetTime()` if `!date.IsValid()`?
>
> AFAIK if `isValid()` is false, then there are no guarantees for the user
> of the library about the internal state.
>
> FWIW the test case is equivalent to
>
>
> void DateTest::testParseDateInvalid()
> {
>        PdfString tmp("D:2012020");
>        PdfDate date(tmp);
>
>        CPPUNIT_ASSERT_EQUAL(false, date.IsValid());
>
>        const time_t t = date.GetTime();
>        CPPUNIT_ASSERT_EQUAL_MESSAGE(time_t(-1), t);
> }
>
> while you asked why it's not
>
> void DateTest::testParseDateInvalid()
> {
>        PdfString tmp("D:2012020");
>        PdfDate date(tmp);
>
>
>        CPPUNIT_ASSERT_EQUAL(false, date.IsValid());
>
>        const time_t t = date.GetTime();
>        CPPUNIT_ASSERT_EQUAL_MESSAGE(time_t(0), t);
> }
>
_______________________________________________
Podofo-users mailing list
Podofo-users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/podofo-users

Reply via email to