ConvertFromPrtime needlessly convert PRTime to microseconds, returning an incorrent icaltimtype

RESOLVED WORKSFORME

Status

--
minor
RESOLVED WORKSFORME
14 years ago
13 years ago

People

(Reporter: mehovis, Assigned: mostafah)

Tracking

Details

(Reporter)

Description

14 years ago
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	{...}
(Reporter)

Comment 1

14 years ago
(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	{...}

(Reporter)

Comment 2

14 years ago
PRTime is already in microseconds,  no need to go the additional three orders of 
magnitude.
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.
(Reporter)

Comment 4

14 years ago
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.
(Reporter)

Updated

14 years ago
Severity: normal → minor
Status: UNCONFIRMED → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → WORKSFORME
Mass move of libxpical bugs to the Internal Components, per ctalbert.
Component: libxpical → Internal Components
The bugspam monkeys have been set free and are feeding on Calendar :: Internal Components. Be afraid for your sanity!
QA Contact: gurganbl → base
You need to log in before you can comment on or make changes to this bug.