<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Fri, Jun 6, 2014 at 3:32 PM, David T. Lewis <span dir="ltr"><<a href="mailto:lewis@mail.msen.com" target="_blank">lewis@mail.msen.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5"><br>
On Fri, Jun 06, 2014 at 11:25:23AM -0700, Eliot Miranda wrote:<br>
><br>
> On Fri, Jun 6, 2014 at 9:33 AM, Douglas McPherson <<a href="mailto:djm1329@san.rr.com">djm1329@san.rr.com</a>><br>
> wrote:<br>
><br>
> ><br>
> ><br>
> > On Jun 5, 2014, at 21:03 , David T. Lewis wrote:<br>
> ><br>
> > > On Thu, Jun 05, 2014 at 06:50:31PM +0200, Bert Freudenberg wrote:<br>
> > >><br>
> > >><br>
> > >> On 04.06.2014, at 18:19, Douglas McPherson <<a href="mailto:djm1329@san.rr.com">djm1329@san.rr.com</a>> wrote:<br>
> > >><br>
> > >>> Hi All,<br>
> > >>><br>
> > >>> I've tested a fix for this on OS X and Linux. The fix simply adds an<br>
> > extra attempt to open the file after the "w+b" attempt using this code:<br>
> > >>><br>
> > >>> if (getFile(f) == NULL) {<br>
> > >>> setFile(f, fopen(cFileName, "wb"));<br>
> > >>> }<br>
> > >>><br>
> > >>> If the previous attempts to open the file failed (because the file<br>
> > exists with write-only permission, no read permission) then the above will<br>
> > actually open the file write-only. Adding the extra attempt preserves<br>
> > existing behaviour for the success paths, so the change should be<br>
> > transparent to existing code.<br>
> > >>><br>
> > >>> BTW, AFAICT the same issue exists in Pharo.<br>
> > >>><br>
> > >>> Thoughts?<br>
> > >>><br>
> > >>> Doug<br>
> > >><br>
> > >> Sounds reasonable to me.<br>
> > >><br>
> > >> - Bert -<br>
> > ><br>
> > > I have not tested it yet, but I think that this would truncate an<br>
> > existing file,<br>
> > > so that if the write-only file exists and contains data, the data would<br>
> > be lost.<br>
> > ><br>
> > > Doug, could you check this with your patched VM?<br>
> > ><br>
> > > Thanks,<br>
> > > Dave<br>
> > ><br>
> ><br>
> > Hi Dave,<br>
> ><br>
> > Yes you are right, I didn't think of that. That is indeed what happens.<br>
> ><br>
> > We could use "ab" instead of "wb" to get append behaviour if the file<br>
> > already exists. I think "a" is still a write-only mode. This is probably<br>
> > safer.<br>
> ><br>
><br>
> OK. Then I propose to commit the following:<br>
><br>
> --- platforms/Cross/plugins/FilePlugin/sqFilePluginBasicPrims.c (revision<br>
> 2888)<br>
> +++ platforms/Cross/plugins/FilePlugin/sqFilePluginBasicPrims.c (working<br>
> copy)<br>
> @@ -245,6 +245,9 @@<br>
> try opening it in write mode to create a new,<br>
> empty file.<br>
> */<br>
> setFile(f, fopen(cFileName, "w+b"));<br>
> + /* and if w+b fails, try ab (not wb which deletes<br>
> read-only files) */<br>
> + if (getFile(f) == NULL)<br>
> + setFile(f, fopen(cFileName, "ab"));<br>
> if (getFile(f) != NULL) {<br>
> char type[4],creator[4];<br>
> dir_GetMacFileTypeAndCreator(sqFileName,<br>
> sqFileNameSize, type, creator);<br>
><br>
<br>
</div></div>This fixes the problem but does not preserve existing behaviour WRT updating<br>
Mac file characteristics, whatever that might happen to mean. Please no rants<br>
about keeping Mac stuff in Cross, but given what we have now I think that a<br>
more compatible fix is:<br>
<div class=""><br>
/* First try to open an existing file read/write: */<br>
setFile(f, fopen(cFileName, "r+b"));<br>
if (getFile(f) == NULL) {<br>
/* Previous call fails if file does not exist. In that case,<br>
try opening it in write mode to create a new, empty file.<br>
*/<br>
setFile(f, fopen(cFileName, "w+b"));<br>
if (getFile(f) != NULL) {<br>
</div> /* New file created, set Mac file characteristics */<br>
<div class=""> char type[4],creator[4];<br>
dir_GetMacFileTypeAndCreator(sqFileName, sqFileNameSize, type, creator);<br>
if (strncmp(type,"BINA",4) == 0 || strncmp(type,"????",4) == 0 || *(int *)type == 0 )<br>
dir_SetMacFileTypeAndCreator(sqFileName, sqFileNameSize,"TEXT","R*ch");<br>
</div> } else { /* if (getFile(f) == NULL) */<br>
/* If the file could not be opened read/write and if a new file<br>
could not be created, then it may be that the file exists but<br>
does not permit read access. Try opening as a write only file,<br>
opened for append to preserve existing file contents.<br>
*/<br>
setFile(f, fopen(cFileName, "ab"));<br>
if (getFile(f) == NULL) {<br>
return interpreterProxy->success(false);<br>
}<br>
}<br>
}<br>
<br>
If no objections, I'll update Eliot's update tomorrow.<br></blockquote><div><br></div><div>Oops. Good catch. Fine with me.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Dave<br>
<br>
</blockquote></div><br><br clear="all"><div><br></div>-- <br>best,<div>Eliot</div>
</div></div>