Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Opening nonexisting file in append mode #26

Open
egri-nagy opened this issue Jan 27, 2015 · 6 comments
Open

Opening nonexisting file in append mode #26

egri-nagy opened this issue Jan 27, 2015 · 6 comments

Comments

@egri-nagy
Copy link

Is this the right behaviour?

gap> IO_CompressedFile("T3.gens", "a");
fail
gap> IO_File("T3.gens", "a");                                       
fail

Checked documentation (up to man 2 open), but no clear answer.

@fingolfin
Copy link
Member

The relevant documentation is not "man 2 open", but rather the IO manual, which says this about the mode parameter to IO_file:
mode must also be a string with possible values "r", "w", or "a", meaning read access, write access (with creating and truncating), and append access respectively.

To me, this says that the answer to your question is "yes, this is intentional".

Of course one may now discuss whether the intent was justified or not ;-). And if we decide it is not, could change things. But before making a change there, I think it's vital to consider ramifications and alternatives, lest this breaks some existing code out there relying on the current behavior...

As to alternatives: IO_File is just a simple wrapper around IO_open + IO_WrapFD. So you could write a function doing what you want yourself, something like this (untested):

CreateAndAppend := function(filename)
    local fd;
    fd := IO_open(filename,IO.O_CREAT+IO.O_APPEND+IO.O_WRONLY,0);
    if fd = fail then return fail; fi;
    return IO_WrapFD(fd,false,IO.DefaultBufSize);
end

As to IO_CompressedFile, I am not sure what appending to a compressed file should do -- that most likely would just produce a broken file anyway, so probably should be forbidden altogether.

@ChrisJefferson
Copy link
Member

Just one minor comment, by design all of gz, bz2 and xz support concatenating different compressed files together and work fine, exactly to support this kind of thing, so having the same functionality on compressed files seem the same.

I would prefer not setting files world-writable by default however, undergraduate students have access to my home directory at uni, I wouldn't want them writing to files I accidentally left world-writable there!

@egri-nagy
Copy link
Author

For the moment just concentrating on the issue critical for me. What would be the potential problem with create and append? This seems to be the right behaviour to me, thinking that in the shell
echo Hello world! >> file
does not fail when file is nonexistent.

I thought about writing something like CreateAndAppend, but first, I call WriteGenerators from the Semigroups package relying on IO_File (sure I could try a pull request there), second that I'm trying to avoid the extra check, since in the computational experiments I call IO_open million times per hour (the penalty of the extra check could be negligible though).

I started with the IO manual, but the quoted sentence does not say anything about the behaviour of mode "a", probably we parse natural language differently :).

Yes, I don't want to break existing code, but I''m also resistant to put glue into downstream code while there seems to be a clean solution upstream.

@ChrisJefferson
Copy link
Member

By the way, having thought about this a little more, I think I tend toward the change. This is because "r", "w" and "a" are (in my mind) typically associated with fopen, and in fopen "a" creates a new file if none exists.

@egri-nagy
Copy link
Author

I also had the same behaviour in my mind.

Then for the second problem of the permissions, would removing world access for appned be sufficient? How about world access for "w"?

@ChrisJefferson
Copy link
Member

Based on your earlier comments, seeing as the permissions are exactly the same as IO's permissions for "w", we should (in this patch at least in my opinion) leave them alone. We could consider, in a separate issue, if it is worth breaking backwards compatibility to lower the default permissions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants