API

[t-w]

There is a lot of discussions already about it, I'd expect more, so maybe we can make a place for it (to list, brainstorm and set directions).

Some ideas to (eventually) consider (please update as needed):

1. file / directory APIs (done)
   • a standard style to implement?
   • changes vs. wrappers
   • issues: fopen() like API for adfFileOpen() #38, Things to finish for 0.8.0 #40
2. consistent naming (ie. submodule prefixes) across the library (done)
3. isolating client API from internal one (separated headers in include/?) (mostly done, but may be improved)
   • eg. for file API, expose only a wrapper over the low level functions(?)
4. devices (done)
   • allow more types of devices (not only one native and "dump"), also custom, user-implemented
   • add a memory/ramdisk device - useful ie. for testing (see the comment below
   • unify the device interface for all types
     • the type of the device is currently is set in adfOpenDev() and depend on the name (ie filename) of the device
       • instead: differentiate opening/creating depending on type (need rather different functions as different arguments are needed, eg. filename for dump, size for ramdisk device)
     • make the same device structure with interface also for dump devices
5. review error codes (see the comment below) and their use (partially done, can be improved)
   • reduce amount of (or remove completely) string output to stdout/err (or leave if a "logger" defined/enabled)
   • replace output warnings with proper return codes
     • bitfield vs enumeration
       • bitfield would allow to aggregate more than one in single return code
       • that was the orig. idea (there is even a test hasRC() - but it is not used anywhere...)
   • user readable (string) error info: logger vs errno/last error info
     • errno/last error info
       • only static error string or dynamic (that would include more info like filename, block
         number or other data relevant for the error)
       • drawbacks: single, global (what can be problematic, even though the lib is not supporting multi-threading)
     • use logger for more detailed error info (if logger enabled/defined)?

Each of these might be a separated issue (or even issues), that could be linked here (so that it is easier to navigate).

[kyz]

I have a few ideas around the "native" interface. I put a simple example in the ramdisk branch, where instead of a single native driver option, it is a linked list, and adfOpenDev() looks at each driver in turn, and there are methods to add/remove a "native driver" from the list. It is then used in tests with a "ramdisk" driver, which is just a block of ram, so no on-disk I/O is needed to check the correctness of FS manipulation, and therefore tests run a little faster.

It is possible to go further with this. There is no need for a native driver slot, and special-casing which one is being used. The adf_dump.c functions can simply be another driver, the default one.

There are then two ways to go. Personally I like the first option, but the second is more explicit.

1. Maintain a chain of potential drivers, each one with a selection function (the "IsDevNative" function, I propose it be called "WillHandle" function, it answers whether the driver will handle the path being opened or not). adfOpenDev() goes through them in turn, calls their WillHandle function, and selects the first one that says yes. There is always one device on the chain, the dump file reading/writing device. Depending on how ADFlib is compiled, there might be two by default, the linux/windows native device reader, which has first priority, and then the dump file reader/writer, which handles everything else.
2. adfOpenDev() requires you pass the device you want, possibly just via a convenience function, e.g. adfNativeDevice() is defined in linux/adf_nativ.c or win32/adf_nativ.c and returns a pointer to a static struct AdfNativeDevice, while adfDumpDevice() is defined in adf_dump.c and returns the same, but for reading from dump files

[kyz]

Should there be more error codes than RC_OK and RC_ERROR?

Well, there are, they're just rarely used. Almost all errors are RC_ERROR. A handful are RC_MALLOC. Two are RC_VOLFULL, and one commented-out line attempts to apply RC_BLOCKSUM

Should we review the errors, and start returning the most appropriate one? Do we want a system like most DOSes where the return code of the function is just yes/no, but the actual error that caused a "no" is available, e.g. dev->lastError, or adfGetLastError(dev), or similar?

Should errors be a bitfield? This is how they're defined, but never used in practise. Or should they just be an enumeration?

[t-w]

Regarding error codes (the last thing left on the above "API improvements list" - still all the things pointed out there are relevant. I am not certain how to improve it yet. Couple more notes about it:

1. There is definitely need for more precise (than only good/bad) error information on many operations (ie. file read/write, opening etc.). Without it, it is difficult to figure out the reason of a failed operation in the client code.
2. The main issue is how to pass the error code, possible ways:
   • error code as a result from the function
     • in the current API, this can be problematic in many cases, since many functions returns pointers, or numbers that do not leave space for encoding there error codes (sizes/offsets), at least without changing int types to at least 64bit
   • a global variable/object with the code of the "last operation" (something like the standard errno)
     • problematic, if the library opens files using ADFlib in different threads (race)
   • extend returned types (structures) to include status (error code, in case of an error), eg. extending struct AdfFile with field status or lastError with the error code of the last operation (but for the current AdfFile object, not a global one; somewhat similar to steam status in C++).
3. Regarding encoding error codes
   • one of the ways could be using the standard errno codes (possibly negated), it would allow easy passing to client code, OS etc. without any "translations"
     • one advantage of negated errno code might be possibility of using it as a result returned from, for instance, adfFileRead() / ..Write() - which could return a positive value (data size read/written) on success, or a negative number representing error code (on error). This would be somewhat similar to standard read() and write(). But this would require the above mentioned change of the int size to big enough to store both things (sizes and error codes).
     • but it would (probably) be good to have also error code more specific for ADFlib, not sure if can "coexist" with errno (which numeric values are not standardized)
     • using errno may be problematic if error states would be encoded in a bitfield (allowing to encode multiple errors eg. on different levels, reading block + (or rather "|", logical or) reading file); this is somewhat they way the errors are encoded now (though not quite used the way they could be...)

(See also notes above, ie. 5. in the issue description)

If there are any thoughts on how to improve this, I am open to discuss this further.

(hopefully, time will come to improve this, finally...)

[rofl0r]

i'd vote for exposing an enum ADFLib_errors or similar from the header and have any function that can fail return one of the error codes contained in it (one of it needs to be ADFLIB_ERROR_SUCCESS or similar ofc), optimally with a documented list of all possible errors each function can return. the success code could use the same value that was previously used for success for backwards compat, however it's more complicated if that value was "true" in the past, not 0, since existing code would then recognize any value except 0 as a success if they used a boolean check.
that solution seems much cleaner than repurposing libc's errno, though if you dont want to break API, storing an error number in the context struct and retrieving it with something like adflib_get_last_error() would be workable too. though api break might not be such a big deal since there's probably less than a handful of users so far, and some of them might use vendored versions embedded into the source tree.