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

[Douglas McPherson]

On Jun 6, 2014, at 15:54 , Eliot Miranda wrote:

> 
> 
> 
> On Fri, Jun 6, 2014 at 3:32 PM, David T. Lewis <lewis at mail.msen.com> wrote:
> 
> 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.
> 
> Oops.  Good catch.  Fine with me.

Yep, good catch. Thanks.

Doug

>  
> 
> Dave
> 
> 
> 
> 
> -- 
> best,
> Eliot

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.squeakfoundation.org/pipermail/vm-dev/attachments/20140606/41df1ade/attachment-0001.htm