fopen() like API for adfFileOpen()

[kyz]

This is a proposal for adfFileOpen() to behave like C's standard fopen():

Mode	Access	Creates file if missing?	Truncates file?	Starting position
r	RO	No	No	Start
r+	R/W	No	No	Start
w	R/W	Yes	Yes	Start
w+	R/W	Yes	Yes	Start
a	R/W	Yes	No	End
a+	R/W	Yes	No	End

What this proposal changes compared to what we had before:

• adfFileOpen(fname, "a") will create a file
• modes r+, w+, a+ are now supported
• mode w and w+ will track the maximum offset written into the file, and if less than the original file size at adfFileClose() time, will truncate the file to your maximum write offset

Truncate-on-close does not follow C fopen semantics, technically it should truncate on open... but that seems to break more tests, I'd have to look at them to find out what assumptions they are making that truncate-on-open violates

[t-w]

I am not particularly fond of adding complexity to this at the moment... especially if this would break any existing functionality. These changes are not entirely trivial and, as you write, break some tests, and, moreover, would actually require testing each of the cases to be sure it really does what is expected.

For me it is a bit overshot - everything can be done without all these additional modes (I would even drop "a", as it is "w" plus seek to the end...). Understanding in details all these modes is more complicated than doing 2 function calls, one for opening the file in write, second to truncate or seek to the end. One basically need the table you prepared to see all those cases, special behaviour (truncate) on close etc... For me, this may be the POSIX standard, but it violates 'S' from 'SOLID' (Single responsibility principle) as open or close do more than just open or close... Truncating the file is more clear if it is done only by the truncate function, not by close or open, or save or whatever. But this is maybe only my view...

So, if you really want to have this POSIX-like complete API (which note that may be confusing if it differs from POSIX in any other place, if, as you wrote yourself before, there is a suggestion that this is a POSIX-compliant API, as it would be with the fopen()-like file open...), my proposition would be to keep this, validate carefully and develop tests and then, when we are sure it does work, we can merge it.

I would prefer to make the release of 0.8.0 before that. Why? Finishing this may take a bit of time, I guess, and we have already postponed this release a very long time. There are many places that can be improved and we cannot do all at once.

For me, the ADFlib has exactly what fuseadf requires it to have (functionally) - it is already working very nicely with read/write support (at least on floppy disk images) and I'd like to tie it to some min. version of the ADFlib. This mod. may require changes/further tests, which is fine, but maybe we could make it for 0.9.0 (if we break the API). We can make it way faster than 0.8.0 (which we will never release if we keep changing things...). I know, I postponed it already quite a bit, but because of the clearly missing functionality, without which write support was just not complete...

I would propose that first we complete the crucial things (patch errors if we see any, esp. critical ones), finish build system config. (i.e. finalize changes for versioning/soname etc.), maybe look at some packaging (if we want to have any artifacts besides a tagged version), so that we can close what is already there and release. And then, having the production pipeline in place, we can make next one in a much shorter cycle, for instance with this complete fopen functionality.

Unless, you think it can be finished soon (in a reliable way). What do you think?

[kyz]

I can be finished very soon indeed.

My main concern is that releasing all this brand new functionality - writing, appending, truncating - with an unfamiliar interface that goes against expectations will lead to user error. And then we'd either change it next release, annoying anyone who started to use this new API, or we'd live with a confusing API that doesn't do what you think it does. Better to get it right first time.

It's not so much that the tests break, it's that the tests reveal undocumented authorial intent, and in this case the author intends that the "a" append mode of a file opening API shouldn't create files... but most file APIs that let you say "a" to append do create files (example: C's fopen, Python's open)

If your concern is logic and complexity, we could adopt the open() style interface instead.

If you're implementing a FUSE backend, you're going to have to translate open() calls anyway. I don't mind what the design is, I went with fopen() because that appeared to be the direction the code was going with string-like open modes.

How about this?

file = adfFileOpen(fname, ADF_RDONLY); // "r"
file = adfFileOpen(fname, ADF_WRONLY | ADF_CREAT | ADF_TRUNC); // "w"
file = adfFileOpen(fname, ADF_WRONLY | ADF_CREAT | ADF_APPEND); // "a"
file = adfFileOpen(fname, ADF_RDWR); // "r+"
file = adfFileOpen(fname, ADF_RDWR | ADF_CREAT | ADF_TRUNC); // "w+"
file = adfFileOpen(fname, ADF_RDWR | ADF_CREAT | ADF_APPEND); // "a+"

And to be clear to users, we can also update the API documentation too, to leave no ambiguity. We can make a decision on each flag.

• Either we document ADF_WRONLY allows reading too, or we make adfFileRead() fail for files opened ADF_WRONLY
• Either we make adfFileWrite() auto-seek to the end of the file if ADF_APPEND is set, or we document ADF_APPEND allows seeking away from the end of the file and it doesn't automatically move back
• We fail adfFileOpen() for flles that don't exist, unless ADF_CREAT is set, in which case we create one, even if the mode is ADF_RDONLY
• ADF_TRUNC truncates immediately on adfFileOpen()

None of this is difficult, I can write all this and update the tests to match

[t-w]

Maybe it is not difficult - but it is a lot of cases to implement, test and document. And all of that is not entirely trivial, either. It is not just "updating tests to match" - these things need tests that will check that functions which behaviour is changing with modes, are working as expected. Also, I am afraid that some gory details may come up and make things not as easy as they seem (I experienced this couple of times already...).

I see that the "a" (append) is the major guilty of the unexpected and surprising behaviour of adfFileOpen(), so for 0.8.0, I will just remove it from open modes. Then, when we figure out how exactly implement all modes (fopen/open or whatever), we can change the API in any way we want without confusing anyone.

Is that OK.?

Note that I do not want to discourage you from implementing these, I really see the point of adding what you propose. But I'd rather prefer that it is done carefully, without rush so that it does not make unnecessary trouble. IMHO, the file API is the most important part of the library, as it will be the most used part (at least, from user's client code perspective). And I agree that it has to be done well. And here we are at the stage of discussing API style to implement...

So can we close 0.8.0 first, implementing just a simple and not confusing API (without "a"), so that I can mark a milestone (basic write support within existing API) and then see how to improve it?

[lclevy]

Ok for me, api without "a" mode for open for 0.8.0