[Vm-dev] [squeak-dev] opening a write-only file

[David T. Lewis]

On Fri, Jun 06, 2014 at 11:25:23AM -0700, Eliot Miranda wrote:
>  
> On Fri, Jun 6, 2014 at 9:33 AM, Douglas McPherson <djm1329 at san.rr.com>
> wrote:
> 
> >
> >
> > On Jun 5, 2014, at 21:03 , David T. Lewis wrote:
> >
> > > On Thu, Jun 05, 2014 at 06:50:31PM +0200, Bert Freudenberg wrote:
> > >>
> > >>
> > >> On 04.06.2014, at 18:19, Douglas McPherson <djm1329 at san.rr.com> wrote:
> > >>
> > >>> Hi All,
> > >>>
> > >>> I've tested a fix for this on OS X and Linux. The fix simply adds an
> > extra attempt to open the file after the "w+b" attempt using this code:
> > >>>
> > >>>         if (getFile(f) == NULL) {
> > >>>            setFile(f, fopen(cFileName, "wb"));
> > >>>         }
> > >>>
> > >>> If the previous attempts to open the file failed (because the file
> > exists with write-only permission, no read permission) then the above will
> > actually open the file write-only. Adding the extra attempt preserves
> > existing behaviour for the success paths, so the change should be
> > transparent to existing code.
> > >>>
> > >>> BTW, AFAICT the same issue exists in Pharo.
> > >>>
> > >>> Thoughts?
> > >>>
> > >>> Doug
> > >>
> > >> Sounds reasonable to me.
> > >>
> > >> - Bert -
> > >
> > > I have not tested it yet, but I think that this would truncate an
> > existing file,
> > > so that if the write-only file exists and contains data, the data would
> > be lost.
> > >
> > > Doug, could you check this with your patched VM?
> > >
> > > Thanks,
> > > Dave
> > >
> >
> > Hi Dave,
> >
> > Yes you are right, I didn't think of that. That is indeed what happens.
> >
> > We could use "ab" instead of "wb" to get append behaviour if the file
> > already exists. I think "a" is still a write-only mode. This is probably
> > safer.
> >
> 
> OK.  Then I propose to commit the following:
> 
> --- platforms/Cross/plugins/FilePlugin/sqFilePluginBasicPrims.c (revision
> 2888)
> +++ platforms/Cross/plugins/FilePlugin/sqFilePluginBasicPrims.c (working
> copy)
> @@ -245,6 +245,9 @@
>                            try opening it in write mode to create a new,
> empty file.
>                         */
>                         setFile(f, fopen(cFileName, "w+b"));
> +                       /* and if w+b fails, try ab (not wb which deletes
> read-only files) */
> +                       if (getFile(f) == NULL)
> +                               setFile(f, fopen(cFileName, "ab"));
>                         if (getFile(f) != NULL) {
>                             char type[4],creator[4];
>                                 dir_GetMacFileTypeAndCreator(sqFileName,
> sqFileNameSize, type, creator);
> 

This fixes the problem but does not preserve existing behaviour WRT updating
Mac file characteristics, whatever that might happen to mean. Please no rants
about keeping Mac stuff in Cross, but given what we have now I think that a
more compatible fix is:

                /* First try to open an existing file read/write: */
                setFile(f, fopen(cFileName, "r+b"));
                if (getFile(f) == NULL) {
                        /* Previous call fails if file does not exist. In that case,
                           try opening it in write mode to create a new, empty file.
                        */
                        setFile(f, fopen(cFileName, "w+b"));
                        if (getFile(f) != NULL) {
                            /* New file created, set Mac file characteristics */
                            char type[4],creator[4];
                                dir_GetMacFileTypeAndCreator(sqFileName, sqFileNameSize, type, creator);
                                if (strncmp(type,"BINA",4) == 0 || strncmp(type,"????",4) == 0 || *(int *)type == 0 )
                                    dir_SetMacFileTypeAndCreator(sqFileName, sqFileNameSize,"TEXT","R*ch");
                        } else { /* if (getFile(f) == NULL) */
                                /* If the file could not be opened read/write and if a new file
                                   could not be created, then it may be that the file exists but
                                   does not permit read access. Try opening as a write only file,
                                   opened for append to preserve existing file contents.
                                */
                                setFile(f, fopen(cFileName, "ab"));
                                if (getFile(f) == NULL) {
                                        return interpreterProxy->success(false);
                                }
                        }
                }

If no objections, I'll update Eliot's update tomorrow.

Dave