132474 - Bad MORK fileopen implementation for OS/2

Status: VERIFIED FIXED

(MailNews Core :: Database, defect)

Description

[Andy Willis (abwillis)]

I have been having a problem here since 0.9.7 with my inbox only working once
after a migration then it comeseup blank.  I found that if I delete the
inbox.msf file it would load clean once then not work the next time.  I was able
to setup a script to delete the inbox.msf on every launch to get around the
problem.  I have this on 2 seperate systems here.  I also made a backup
directory of my inbox(inbox2) and it works everytime.  I then deleted the inbox
and inbox.msf and copied the inbox2 and
inbox2.msf over to inbox and inbox.msf respectively and did the same in netscape
to let it get converted.  Same problem but always the inbox2 works.  I have
created inbox2 by coping the files and by creating a folder and coping the files
over inside warpzilla.
When the inbox is empty the default text for the inbox is not whatever it should
be - mine should have been Western but kept on showing as Arabic. Also the inbox
when it is correct has only one tab in properties showing Western.  When it is
empty the properties has 3 tabs, the second like that of a newsgroup the third
is sharing.  The closest bug I found was 48955 but not sure if it is the same or
not.
 Andy

Comment 1

[Mike Kaply [:mkaply]]

Patch coming up.

Comment 2

[Mike Kaply [:mkaply]]

Attachment: Custom MORK_FILEOPEN for OS/2

Mork was making the BAD assumption that two subsequent fopens on a file succeed
all the time. I had to rewrite it for OS/2.

This involved one XP thing. Mork already had #defines in morkConfig.h that
mapped the file APIs to platform stuff. For some reason they left out fopen. I
fixed that.

I also changed the #defines in morkConfig.h - XP_WIN was being defined if XP_PC
was set which is wrong. This was causing XP_WIN to be defined for XP_OS2.

Then I implemented XP_FILEOPEN for OS/2 using sopen and fdopen.

Works like a charm.

This is CRUCIAL for OS/2. It fixes mail.

Comment 3

[David :Bienvenu]

that all makes sense, but why add duplicates of FSEEK, FTELL, and FFLUSH? Oh
well, I guess it doesn't really matter - thanks for fixing this.

Comment 4

[David :Bienvenu]

Comment on attachment 75474 [details] [diff] [review]
Custom MORK_FILEOPEN for OS/2

r=bienvenu

Comment 5

[Mike Kaply [:mkaply]]

Attachment: New diff

You are right. I should just do the OS/2 within the bigger #ifdef.

Originally I was going to reimplement them all as OS/2 APIs until I saw I could
use sopen and fopen.

Can I get an r= on this? I really want to get this in.

Comment 6

[David :Bienvenu]

Comment on attachment 75479 [details] [diff] [review]
New diff

r=bienvenu (if you want that to be an sr, that's fine with me, either way). I
don't care who the other reviewer is.

Comment 7

[Mike Kaply [:mkaply]]

*** Bug 127493 has been marked as a duplicate of this bug. ***

Comment 8

[Mike Kaply [:mkaply]]

Attachment: Last patch I swear

I thought things through over dinner and I don't think the implementation of
mork_fileopen belongs in a header, given that at some point, MORK_FILEOPEN
could be used in some other C file.

morkConfig.cpp seems the perfect place for this function.

This is essentially the same diff with things moved to morkConfig.

BTW, I'll take your review as a superreview and get one of my team to review
it.

Thanks for all the help. 

Sorry for all the patches :)

Comment 9

[David :Bienvenu]

-> Michael, thx for fixing this.

Comment 10

[(not reading, please use [e-mail] instead)]

sr=sspitzer, once it has r=bienvenu

Comment 11

[Mike Kaply [:mkaply]]

Comment on attachment 75525 [details] [diff] [review]
Last patch I swear

per note from sspitzer, bienvenu had reviewed it (last patch isn't any code
change from the second, just refactoring.

Comment 12

[Asa Dotzler [:asa]]

Comment on attachment 75525 [details] [diff] [review]
Last patch I swear

a=asa (on behalf of drivers) for checkin to the 1.0 trunk

Comment 13

[Mike Kaply [:mkaply]]

Fix checked in

Comment 14

[Mike Kaply [:mkaply]]

*** Bug 132955 has been marked as a duplicate of this bug. ***

Comment 15

[jhp (no longer active)]

Reopening bug:  Patch is not complete.  We need to specify the pmode argument to
sopen() when passing in the O_CREAT flag.

Comment 16

[jhp (no longer active)]

Attachment: pmode patch

Comment 17

[David :Bienvenu]

Comment on attachment 76080 [details] [diff] [review]
pmode patch

r=bienvenu

Comment 18

[Mike Kaply [:mkaply]]

New fix already went in as bustage fix.

Comment 19

[Mike Kaply [:mkaply]]

marking verified.

Everyone seems happy with mail now.