264058 - ConvertFromPrtime needlessly convert PRTime to microseconds, returning an incorrent icaltimtype

Status: RESOLVED WORKSFORME

(Calendar :: Internal Components, defect)

Description

[Michael Hovis]

User-Agent:       Mozilla/5.0 (Windows NT 5.1; U) Opera 7.54  [en]
Build Identifier: Revision 1.87 of oeICalEventImpl.cpp used in XulRunner branch

icaltimetype ConvertFromPrtime( PRTime indate ) {
    icaltimetype outdate = icaltime_null_time();

    PRExplodedTime ext;
    // Not required, PR_ExplodeTime expects PRTime as returned from PR_Now
    // not something in microseconds.
    //-PRInt64 indateinusec, usecpermsec;
    //-LL_I2L( usecpermsec, PR_USEC_PER_MSEC );
    //-LL_MUL( indateinusec, indate, usecpermsec );
    //-PR_ExplodeTime( indateinusec, PR_LocalTimeParameters, &ext);
 
    outdate.year = ext.tm_year;
    outdate.month = ext.tm_month + 1;
    outdate.day = ext.tm_mday;
    outdate.hour = ext.tm_hour;
    outdate.minute = ext.tm_min;
    outdate.second = ext.tm_sec;
    if( ext.tm_year <= 1969 && ext.tm_params.tp_dst_offset == 3600 )//Assume 
that daylight time saving was not in effect before 1970
        icaltime_adjust( &outdate, 0, -1, 0, 0 );

    return outdate;
}

Reproducible: Always
Steps to Reproduce:
1.Create an instance of an oeIICalEvent and get the oeIDateTime start value.
2.Set a PRTime variable to PR_Now();
3.Pass the PRTime variable to SetTime() in the oeIDateTime instance and look at 
what is stored( nonsense date ).

Actual Results:  
indate	1097602775677250
indateinusec	1097602775677250000
-	ext	{...}
	tm_usec	250000
	tm_sec	37
	tm_min	54
	tm_hour	13
	tm_mday	7
	tm_month	11
	tm_year	-28786
	tm_wday	6 ''
	tm_yday	340




Expected Results:  
	indate	1097602775677250
-	ext	{...}
	tm_usec	677250
	tm_sec	35
	tm_min	39
	tm_hour	12
	tm_mday	12
	tm_month	9
	tm_year	2004
	tm_wday	2 ''
	tm_yday	285
+	tm_params	{...}

Comment 1

[Michael Hovis]

(In reply to comment #0)
> User-Agent:       Mozilla/5.0 (Windows NT 5.1; U) Opera 7.54  [en]
> Build Identifier: Revision 1.87 of oeICalEventImpl.cpp used in XulRunner 
branch
> 
> icaltimetype ConvertFromPrtime( PRTime indate ) {
>     icaltimetype outdate = icaltime_null_time();
> 
>     PRExplodedTime ext;
>     // Not required, PR_ExplodeTime expects PRTime as returned from PR_Now
>     // not something in microseconds.
>     //-PRInt64 indateinusec, usecpermsec;
>     //-LL_I2L( usecpermsec, PR_USEC_PER_MSEC );
>     //-LL_MUL( indateinusec, indate, usecpermsec );
>     PR_ExplodeTime( indateinusec, PR_LocalTimeParameters, &ext);
>  
>     outdate.year = ext.tm_year;
>     outdate.month = ext.tm_month + 1;
>     outdate.day = ext.tm_mday;
>     outdate.hour = ext.tm_hour;
>     outdate.minute = ext.tm_min;
>     outdate.second = ext.tm_sec;
>     if( ext.tm_year <= 1969 && ext.tm_params.tp_dst_offset == 3600 )//Assume 
> that daylight time saving was not in effect before 1970
>         icaltime_adjust( &outdate, 0, -1, 0, 0 );
> 
>     return outdate;
> }
> 
> Reproducible: Always
> Steps to Reproduce:
> 1.Create an instance of an oeIICalEvent and get the oeIDateTime start value.
> 2.Set a PRTime variable to PR_Now();
> 3.Pass the PRTime variable to SetTime() in the oeIDateTime instance and look 
at 
> what is stored( nonsense date ).
> 
> Actual Results:  
> indate	1097602775677250
> indateinusec	1097602775677250000
> -	ext	{...}
> 	tm_usec	250000
> 	tm_sec	37
> 	tm_min	54
> 	tm_hour	13
> 	tm_mday	7
> 	tm_month	11
> 	tm_year	-28786
> 	tm_wday	6 ''
> 	tm_yday	340
> 
> 
> 
> 
> Expected Results:  
> 	indate	1097602775677250
> -	ext	{...}
> 	tm_usec	677250
> 	tm_sec	35
> 	tm_min	39
> 	tm_hour	12
> 	tm_mday	12
> 	tm_month	9
> 	tm_year	2004
> 	tm_wday	2 ''
> 	tm_yday	285
> +	tm_params	{...}

Comment 2

[Michael Hovis]

PRTime is already in microseconds,  no need to go the additional three orders of 
magnitude.

Comment 3

[Michiel van Leeuwen (email: mvl+moz@)]

Calendar uses PRTime, but stores time as miliseconds. (because this is how you
get a time from javascript)
So the code is correct, but you can argue that the wrong datatypes are used.

Comment 4

[Michael Hovis]

Yeah, the severity is quite low, I just pass my PRTime variable in  (when called
within c++ )  / 1000 to workaround the bug - no need to have microsecond
resolution anyhow.

Comment 5

[Marcia Knous [:marcia]]

Mass move of libxpical bugs to the Internal Components, per ctalbert.

Comment 6

[Reed Loden [:reed]]

The bugspam monkeys have been set free and are feeding on Calendar :: Internal Components. Be afraid for your sanity!