72014 - DBM temporary file problems on OS/2

Status: VERIFIED FIXED

(SeaMonkey :: General, defect, P2)

Description

[Nelson Bolyard (seldom reads bugmail)]

All the problems reported against DBM on Windows in bug 69753
also affect DBM on OS/2.  Those problems have been fixed for 
windows, but not for OS/2.  Please see bug 69753 for the full
description and the patches.

Comment 1

[Nelson Bolyard (seldom reads bugmail)]

Michael Kaply <[e-mail]> might be willing to work on this bug.

Comment 2

[jhp (no longer active)]

Attachment: patch based on 69753

Comment 3

[jhp (no longer active)]

This seems to be the only necessary change. Please let me know if I 
missed something (OS/2 does not have a TEMPORARY flag for open()).

Also, I liked your suggestion for rewriting gettemp.  I just looked 
and OS/2 contains several functions that create and manipulate 
temporary files, which are deleted once the process ends.  Are you 
still thinking of doing this?

Comment 4

[jhp (no longer active)]

Adding myself to cc.

Comment 5

[Robert Relyea]

Is this change just to get OS/2 to compile? It seems that this will still leave
temparary files around (though that's what the old code did as well). If you
have an OS/2 specific function that can open a temp file which automatically
gets deleted on process end or on close that would be preferable.

Nelson do we need a superreview on this (because it's DBM code?).

bob

Comment 6

[jhp (no longer active)]

The OS/2 code compiles fine with the patches from 69753.  This patch 
is just to get the temp file opened as a binary, whereas it was 
defaulting to text before.

I mentioned the temp file function because nelsonb had said something 
about it in 69753.  In order to use it, though, would require a 
rewrite of mktemp.c.

Comment 7

[Nelson Bolyard (seldom reads bugmail)]

Javier and I spoke today.  As I understand it, OS/2 has a function
that takes no arguments and returns a FILE * for a temporary file
that is opened binary read/write, and that will disappear when the 
program terminates.  If it is possible to extract the fd from the 
FILE, then it should be possible to rewrite the function 
mkstempflags in mktemp.c to use this OS/2 function, ignoring the
input arguments.  I recoomend that.

Comment 8

[jhp (no longer active)]

Attachment: patch using tmpfile()

Comment 9

[jhp (no longer active)]

Ok, so ignore the first patch.  This second patch include a rewrite of 
mkstemp, which calls the tmpfile() function.  The tmpfile() function 
creates a temporary file which is deleted once the program has 
finished executing.  I also rewrote h_page.c to make use of this 
function for os/2.

I still need to study if the filename is actually used.  The function 
tmpfile() does not return a filename.  For this reason, I also need to 
keep around mktemp and gettemp (which return filenames), because 
mktemp is used in the jpeg code.

Comment 10

[jhp (no longer active)]

Attachment: better patch

Comment 11

[jhp (no longer active)]

Created a better patch that bypasses most of open_temp() and just sets 
the hash fd.

Comment 12

[jhp (no longer active)]

Attachment: fixed endif comment

Comment 13

[Robert Relyea]

OK, Javier, I like the patch, are we ready for a superreview?

bob

Comment 14

[jhp (no longer active)]

yes, i think it's ready to go...

Comment 15

[Mike Kaply [:mkaply]]

Who is the right sr for this?

Comment 16

[Christopher Blizzard (:blizzard)]

sr=blizzard

Comment 17

[Nelson Bolyard (seldom reads bugmail)]

Is this bug fixed now?

Comment 18

[Asa Dotzler [:asa]]

a= [e-mail] for checkin to the trunk.
(on behalf of drivers)

Comment 19

[Christopher Blizzard (:blizzard)]

Just to clarify this is approved for 0.9.2 too.

Comment 20

[Wan-Teh Chang]

I checked in Javier's last patch on the trunk (edited
to imitate the existing coding style of the modified
files).  Since the tree is supposed to branch for 0.9.2
tomorrow morning, this fix will be in 0.9.2 as well.

Comment 21

[Wan-Teh Chang]

Attachment: Javier's patch, edited and checked in by wtc.

Comment 22

[Wan-Teh Chang]

I am changing the product to Browser because mozilla/dbm
is not part of NSS.  I am setting target milestone to
mozilla0.9.2.

Comment 23

[Mike Kaply [:mkaply]]

verify fixed by looking at code changes.