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.

Sign up for GitHub

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

Add in-memory I/O using hFILE fixed buffers #590

Closed
wants to merge 9 commits into from
Closed

Add in-memory I/O using hFILE fixed buffers #590

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
80bb84f
Add in-memory I/O
ThomasHickman Sep 11, 2017
58fb979
Add better error handling in hpreload_fd
ThomasHickman Sep 12, 2017
29a6789
Add create_hfile_mem
ThomasHickman Sep 12, 2017
5cd9e07
Move mem backend code to the correct section
ThomasHickman Sep 12, 2017
3972015
Fix issues with mem hfile
ThomasHickman Sep 12, 2017
3d11377
Free hFILE buffer when erroring
ThomasHickman Sep 13, 2017
3656732
Stop freeing hfile mem buffers in hclose
ThomasHickman Sep 22, 2017
fde4dcf
Fixes to PR#590.
jkbonfield Dec 7, 2017
5fa67e9
Create preload scheme
ThomasHickman Dec 12, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
94 changes: 92 additions & 2 deletions hfile.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,7 @@ off_t hseek(hFILE *fp, off_t offset, int whence)
{
off_t curpos, pos;

if (writebuffer_is_nonempty(fp)) {
if (writebuffer_is_nonempty(fp) && fp->mobile) {
int ret = flush_buffer(fp);
if (ret < 0) return ret;
}
Expand Down Expand Up @@ -615,6 +615,39 @@ static hFILE *hopen_fd(const char *filename, const char *mode)
return NULL;
}

static hFILE *hpreload_fd(const char *filename, const char *mode)
{
if(!strchr(mode, 'r'))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should check mode value. If NULL, strchr will seg fault.

{
return NULL;
}

hFILE_fd *fp = NULL;
FILE *file = fopen(filename, mode);
if (!file) goto error;

if(fseek(file, 0, SEEK_END) != 0) goto error;
int len = ftell(file);
fseek(file, 0, SEEK_SET);

char* buffer = malloc(len);
if(buffer == NULL) goto error;
if(fread(buffer, 1, len, file) != len) goto error;

fp = (hFILE_fd *) hfile_init_fixed(sizeof (hFILE_fd), mode, buffer, len, len);
if (fp == NULL) goto error;

fp->fd = fileno(file);
fp->is_socket = 0;
fp->base.backend = &fd_backend;
return &fp->base;

error:
if (file) { int save = errno; (void) fclose(file); errno = save; }
hfile_destroy((hFILE *) fp);
return NULL;
}

hFILE *hdopen(int fd, const char *mode)
{
hFILE_fd *fp = (hFILE_fd*) hfile_init(sizeof (hFILE_fd), mode, blksize(fd));
Expand Down Expand Up @@ -711,6 +744,15 @@ static int cmp_prefix(const char *key, const char *s)
return 0;
}

static hFILE *create_hfile_mem(char* buffer, const char* mode, size_t buf_filled, size_t buf_size)
{
hFILE_mem *fp = (hFILE_mem *) hfile_init_fixed(sizeof(hFILE_mem), mode, buffer, buf_filled, buf_size);
if (fp == NULL) { free(buffer); return NULL; }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should let the caller function free buffer.


fp->base.backend = &mem_backend;
return &fp->base;
}

static hFILE *hopen_mem(const char *url, const char *mode)
{
size_t length, size;
Expand All @@ -735,6 +777,8 @@ static hFILE *hopen_mem(const char *url, const char *mode)
hts_decode_percent(buffer, &length, data);
}

return create_hfile_mem(buffer, mode, length, size);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You still need to free buffer. I would check the value returned by create_hfile_mem and, if NULL, free(buffer).


hFILE_mem *fp = (hFILE_mem *)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code is unreachable.

hfile_init_fixed(sizeof (hFILE_mem), mode, buffer, length, size);
if (fp == NULL) { free(buffer); return NULL; }
Expand All @@ -743,6 +787,37 @@ static hFILE *hopen_mem(const char *url, const char *mode)
return &fp->base;
}

hFILE *hopenv_mem(const char *filename, const char *mode, va_list args)
{
char* buffer = va_arg(args, char*);
size_t sz = va_arg(args, size_t);
va_end(args);

return create_hfile_mem(buffer, mode, sz, sz);
}

int hfile_mem_get_buffer(hFILE *file, char **buffer, size_t *length){
if(file->backend != &mem_backend) {
errno = EINVAL;
return -1;
}

*buffer = file->buffer;
*length = file->buffer - file->limit;

return 0;
}

int hfile_plugin_init_mem(struct hFILE_plugin *self)
{
// mem files are declared remote so they work with a tabix index
static const struct hFILE_scheme_handler handler =
{NULL, hfile_always_remote, "mem", 2000 + 50, hopenv_mem};
self->name = "mem";
hfile_add_scheme_handler("mem", &handler);
return 0;
}


/*****************************************
* Plugin and hopen() backend dispatcher *
Expand Down Expand Up @@ -833,6 +908,7 @@ static void load_hfile_plugins()
hfile_add_scheme_handler("data", &data);
hfile_add_scheme_handler("file", &file);
init_add_plugin(NULL, hfile_plugin_init_net, "knetfile");
init_add_plugin(NULL, hfile_plugin_init_mem, "mem");

#ifdef ENABLE_PLUGINS
struct hts_path_itr path;
Expand Down Expand Up @@ -879,11 +955,25 @@ static hFILE *hopen_unknown_scheme(const char *fname, const char *mode)
return fp;
}

static hFILE *hopenv_unknown_scheme(const char *fname, const char *mode, va_list args)
{
char* method_type = va_arg(args, char*);
va_end(args);
if(!strcmp(method_type, "preload")){
errno = EPROTONOSUPPORT;
return NULL;
}

hFILE *fp = hpreload_fd(fname, mode);
if (fp == NULL && errno == ENOENT) errno = EPROTONOSUPPORT;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm unsure why this line exists. hpreload_fd can return NULL/ENOENT for valid reasons, and ENOENT is suitable then I think.

return fp;
}

/* Returns the appropriate handler, or NULL if the string isn't an URL. */
static const struct hFILE_scheme_handler *find_scheme_handler(const char *s)
{
static const struct hFILE_scheme_handler unknown_scheme =
{ hopen_unknown_scheme, hfile_always_local, "built-in", 0 };
{ hopen_unknown_scheme, hfile_always_local, "built-in", 2000 + 50, hopenv_unknown_scheme };

char scheme[12];
int i;
Expand Down
17 changes: 15 additions & 2 deletions htslib/hfile.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ hread(hFILE *fp, void *buffer, size_t nbytes)
if (n > nbytes) n = nbytes;
memcpy(buffer, fp->begin, n);
fp->begin += n;
return (n == nbytes)? (ssize_t) n : hread2(fp, buffer, nbytes, n);
return (n == nbytes || !fp->mobile)? (ssize_t) n : hread2(fp, buffer, nbytes, n);
}

/// Write a character to the stream
Expand Down Expand Up @@ -239,7 +239,15 @@ static inline ssize_t HTS_RESULT_USED
hwrite(hFILE *fp, const void *buffer, size_t nbytes)
{
extern ssize_t hwrite2(hFILE *, const void *, size_t, size_t);

extern int hfile_set_blksize(hFILE *fp, size_t bufsiz);

if(!fp->mobile){
if (fp->limit - fp->begin < nbytes){
hfile_set_blksize(fp, fp->limit - fp->buffer + nbytes);
fp->end = fp->limit;
}
}

size_t n = fp->limit - fp->begin;
if (n > nbytes) n = nbytes;
memcpy(fp->begin, buffer, n);
Expand All @@ -254,6 +262,11 @@ This includes low-level flushing such as via `fdatasync(2)`.
*/
int hflush(hFILE *fp) HTS_RESULT_USED;

/// For hfile_mem: get the internal buffer and it's size from a hfile
/** @return 0 if successful, or -1 if an error occurred
*/
int hfile_mem_get_buffer(hFILE *file, char **buffer, size_t *length);

#ifdef __cplusplus
}
#endif
Expand Down
32 changes: 32 additions & 0 deletions test/hfile.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,38 @@ int main(void)
if ((c = hgetc(fin)) != EOF) fail("chars: hgetc (EOF) returned %d", c);
if (hclose(fin) != 0) fail("hclose(test/hfile_chars.tmp) for reading");

fin = hopen("test/hfile_chars.tmp", "r:", "preload");
if (fin == NULL) fail("preloading hopen(\"test/hfile_chars.tmp\") for reading");
for (i = 0; i < 256; i++)
if ((c = hgetc(fin)) != i)
fail("preloading chars: hgetc (%d = 0x%x) returned %d = 0x%x", i, i, c, c);
if ((c = hgetc(fin)) != EOF) fail("preloading chars: hgetc (EOF) returned %d", c);
if (hclose(fin) != 0) fail("preloading hclose(test/hfile_chars.tmp) for reading");

char* test_string = strdup("Test string");
fin = hopen("mem:", "r:", test_string, 12);
if (fin == NULL) fail("hopen(\"mem:\", \"r:\", ...)");
if (hread(fin, buffer, 12) != 12)
fail("hopen('mem:', 'r') failed read");
if(strcmp(buffer, test_string) != 0)
fail("hopen('mem:', 'r') missread '%s' != '%s'", buffer, test_string);
if (hclose(fin) != 0) fail("hclose mem for reading");

test_string = strdup("Test string");
fin = hopen("mem:", "wr:", test_string, 12);
if (fin == NULL) fail("hopen(\"mem:\", \"w:\", ...)");
if (hseek(fin, -1, SEEK_END) < 0)
fail("hopen('mem:', 'wr') failed seek");
if (hwrite(fin, strdup(" extra"), 7) != 7)
fail("hopen('mem:', 'wr') failed write");
if (hseek(fin, 0, SEEK_SET) < 0)
fail("hopen('mem:', 'wr') failed seek");
if (hread(fin, buffer, 18) != 18)
fail("hopen('mem:', 'wr') failed read");
if (strcmp(buffer, "Test string extra") != 0)
fail("hopen('mem:', 'wr') misswrote '%s' != '%s'", buffer, "Test string extra");
if (hclose(fin) != 0) fail("hclose mem for writing");

fin = hopen("data:,hello, world!%0A", "r");
if (fin == NULL) fail("hopen(\"data:...\")");
n = hread(fin, buffer, 300);
Expand Down