149163 - x11shared and file stat

Status: RESOLVED WONTFIX

(Core :: Graphics: Text, defect)

Description

[Jon Smirl]

The code building the the font cache is using two different stats and then 
comparing the results. This works on some systems but not all.

When the font cache is written the font file timestamp is retrieved via the 
XPCOM localfile support which uses stat internally. Later when it comes time to 
compare the fonts file timestamps the stat from stdlib.h is being used 
directly. This leads to portability problems with whether stat is returning 
milli/micro seconds. The better solution is to always use the nsLocalFile calls 
for retrieving file creation times and to not use stdlib.h.

FT2Catalog probably shouldn't include any of these files:
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <sys/stat.h>
#include <sys/time.h>

Removing them all on my system only resulted in minor changes to the code.

Comment 1

[John Morrison]

-> bstell

Comment 2

[kill this account]

Jon: I know you are working on this already. Do you have patches for this?

Comment 3

[Jon Smirl]

I have already hacked the files up removing GTX and XWindows dependencies so I 
can't generate diffs of only the stat code.  Just remove the stdlib.h header 
and fix the compiler errors. There are only two places. I'll attach my full 
version.

I also modified that code that writes the times out to write a 64bit field. I 
was having trouble with the leading digits getting truncated. Is the format 
%I64u portable?

The time from stat is in milliseconds on some systems and microseconds on 
other. NSPR hides this from you. Mixing direct stat usuage with NSPR exposed 
this.

Comment 4

[Jon Smirl]

Attachment: version of catalog with stat issue fixed

Comment 5

[kill this account]

Jon: normally people review diffs.
Would you kindly attach a "diff -u" ?

Comment 6

[Jon Smirl]

I attached a diff. I am not running Linux so I can't verify if this compiles. 
It does work with my version running on windows.

Comment 7

[Jon Smirl]

Attachment: diffs for portability changes

Comment 8

[Jon Smirl]

Patch also fixes 149160 

Comment 9

[kill this account]

> I am not running Linux so I can't verify if this compiles. 

I hope you will understand that I strongly feel this should not even be
reviewed until it is compiled and tested on a Linux/Unix system.

Comment 10

[Ben Bucksch (:BenB)]

Brian Stell, if he has no Linux system, you cannot ask that. Testing on other
systems can be part of the review.
Please confirm bugs with good patches. Doing so.

Comment 11

[kill this account]

Please correct me if I am wrong but it is my understanding that it has been the 
long time standing policy to for developers to get "build buddies" to help test 
their patches if the patch is likely to affect other platforms.

Comment 12

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee didn't login in Bugzilla in the last 7 months.
:enndeakin, could you have a look please?
For more information, please visit auto_nag documentation.