ImagingDataCommons / libdicom

C library for reading DICOM files
https://libdicom.readthedocs.io
MIT License
16 stars 7 forks source link

Add pluggable IO #36

Closed jcupitt closed 1 year ago

jcupitt commented 1 year ago

This (huge) PR rewrites dicom-file.c to do all IO using just open/close/read/seek functions (so no more stdio).

It adds dicom-io.c with implementations for file read and read from memory buffer.

The file read functions use input buffering, so speed should be good.

There's a test for load from memory area.

I renamed dcm_file_create() as dcm_file_open() and added dcm_memory_open(). I felt dcm_file_create() sounded too much like creating a file on disc.

There's still dcm_file_create_io() to create a DcmFile object from a set of user open/close/read/seek functions.

jcupitt commented 1 year ago

OK @hackermd, I think this is ready for review. Though I don't think detailed review is needed at this stage, just a quick check it doesn't look crazy.

hackermd commented 1 year ago

I renamed dcm_file_create() as dcm_file_open() and added dcm_memory_open(). I felt dcm_file_create() sounded too much like creating a file on disc.

Good point! I kinda like the consistent _create() and _destroy() pattern for the "constructor" and "destructor" functions, respectively. Maybe we should instead rename DcmFile to DcmFileHandle and dcm_file_ to dcm_filehandle_?

jcupitt commented 1 year ago

Good point! I kinda like the consistent _create() and _destroy() pattern for the "constructor" and "destructor" functions,

They are kind-of still there. The main constructor is dcm_file_create_io():

DCM_EXTERN
DcmFile *dcm_file_create_io(DcmError **error, DcmIO *io, void *client);

The DcmIO struct is a set of four functions for IO:

typedef struct _DcmIO {
    void *(*open)(DcmError **error, void *client);
    int (*close)(DcmError **error, void *data);
    int64_t (*read)(DcmError **error, void *data, char *buffer, int64_t length);
    int64_t (*seek)(DcmError **error, void *data, int64_t offset, int whence);
} DcmIO;

Then dcm_file_open() is a tiny wrapper over dcm_file_create_io() with funcs for doing IO on files on disc:

DcmFile *dcm_file_open(DcmError **error, const char *file_path)
{
    static DcmIO io = {
        dcm_io_open_file,
        dcm_io_close_file,
        dcm_io_read_file,
        dcm_io_seek_file,
    };

    return dcm_file_create_io(error, &io, (void *) file_path);
}

And dcm_file_memory() is exactly the same, but with a set of funcs for loading from an area of memory.

jcupitt commented 1 year ago

@hackermd could you approve this? The other two PR depend on this one.

jcupitt commented 1 year ago

Thanks!