From 4dfa55222d1fe433a6f5e0e8c67b1168a9cb1ee3 Mon Sep 17 00:00:00 2001 From: Arnar Mar Hrafnkelsson Date: Wed, 31 Oct 2001 17:48:46 +0000 Subject: [PATCH] Added io_buffer for reading from scalars. Also added test cases. Added a myfree() call in XS code for gsamp. Switched tags code to use mymalloc/free which found a bufferover run in some gif tag code (test fails). Fixed some mymalloc/free bugs in bmp.c and tga.c. --- Imager.xs | 26 ++++++- bmp.c | 6 +- iolayer.c | 203 ++++++++++++++++++++++++++++++++++++++++++++++--- iolayer.h | 14 +++- t/t07iolayer.t | 67 ++++++++++++++++ tags.c | 18 ++--- tga.c | 2 +- 7 files changed, 306 insertions(+), 30 deletions(-) create mode 100644 t/t07iolayer.t diff --git a/Imager.xs b/Imager.xs index 066e9cfd..3de360f0 100644 --- a/Imager.xs +++ b/Imager.xs @@ -28,6 +28,11 @@ typedef TT_Fonthandle* Imager__TTHandle; typedef FT2_Fonthandle* Imager__Font__FT2; #endif + +void my_SvREFCNT_dec(void *p) { + SvREFCNT_dec((SV*)p); +} + typedef struct i_reader_data_tag { /* presumably a CODE ref or name of a sub */ @@ -674,17 +679,29 @@ Imager::IO io_new_bufchain() +Imager::IO +io_new_buffer(data) + char *data + PREINIT: + size_t length; + SV* sv; + CODE: + SvPV(ST(0), length); + SvREFCNT_inc(ST(0)); + RETVAL = io_new_buffer(data, length, my_SvREFCNT_dec, ST(0)); + OUTPUT: + RETVAL + + void io_slurp(ig) Imager::IO ig PREINIT: unsigned char* data; - size_t tlength; - SV* r; + size_t tlength; PPCODE: data = NULL; tlength = io_slurp(ig, &data); - r = sv_newmortal(); EXTEND(SP,1); PUSHs(sv_2mortal(newSVpv(data,tlength))); myfree(data); @@ -2589,8 +2606,9 @@ i_gsamp(im, l, r, y, ...) chans = mymalloc(sizeof(int) * chan_count); for (i = 0; i < chan_count; ++i) chans[i] = SvIV(ST(i+4)); - data = mymalloc(sizeof(i_sample_t) * (r-l) * chan_count); + data = mymalloc(sizeof(i_sample_t) * (r-l) * chan_count); /* XXX: memleak? */ count = i_gsamp(im, l, r, y, data, chans, chan_count); + myfree(chans); if (GIMME_V == G_ARRAY) { EXTEND(SP, count); for (i = 0; i < count; ++i) diff --git a/bmp.c b/bmp.c index fbc09236..533fdbc9 100644 --- a/bmp.c +++ b/bmp.c @@ -760,8 +760,8 @@ read_4bit_bmp(io_glue *ig, int xsize, int ysize, int clr_used, break; case BMPRLE_ENDOFBMP: - free(packed); - free(line); + myfree(packed); + myfree(line); return im; case BMPRLE_DELTA: @@ -889,7 +889,7 @@ read_8bit_bmp(io_glue *ig, int xsize, int ysize, int clr_used, break; case BMPRLE_ENDOFBMP: - free(line); + myfree(line); return im; case BMPRLE_DELTA: diff --git a/iolayer.c b/iolayer.c index 7089ffec..cdcb2ff3 100644 --- a/iolayer.c +++ b/iolayer.c @@ -193,6 +193,132 @@ realseek_seek(io_glue *ig, off_t offset, int whence) { + + + + + + + + + + + + + + + + +/* + * Callbacks for sources that are a fixed size buffer + */ + +/* +=item buffer_read(ig, buf, count) + +Does the reading from a buffer source + + ig - io_glue object + buf - buffer to return data in + count - number of bytes to read into buffer max + +=cut +*/ + +static +ssize_t +buffer_read(io_glue *ig, void *buf, size_t count) { + io_ex_buffer *ieb = ig->exdata; + char *cbuf = buf; + + IOL_DEB( printf("buffer_read: fd = %d, ier->cpos = %ld, buf = %p, count = %d\n", fd, (long) ier->cpos, buf, count) ); + + if ( ieb->cpos+count > ig->source.buffer.len ) { + mm_log((1,"buffer_read: short read: cpos=%d, len=%d, count=%d\n", ieb->cpos, ig->source.buffer.len)); + count = ig->source.buffer.len - ieb->cpos; + } + + memcpy(buf, ig->source.buffer.data+ieb->cpos, count); + ieb->cpos += count; + IOL_DEB( printf("buffer_read: rc = %d, count = %d\n", rc, count) ); + return count; +} + + +/* +=item buffer_write(ig, buf, count) + +Does nothing, returns -1 + + ig - io_glue object + buf - buffer that contains data + count - number of bytes to write + +=cut +*/ + +static +ssize_t +buffer_write(io_glue *ig, const void *buf, size_t count) { + mm_log((1, "buffer_write called, this method should never be called.\n")); + return -1; +} + + +/* +=item buffer_close(ig) + +Closes a source that can be seeked on. Not sure if this should be an actual close +or not. Does nothing for now. Should be fixed. + + ig - data source + +=cut +*/ + +static +void +buffer_close(io_glue *ig) { + mm_log((1, "buffer_close(ig %p)\n", ig)); + /* FIXME: Do stuff here */ +} + + +/* buffer_seek(ig, offset, whence) + +Implements seeking for a buffer source. + + ig - data source + offset - offset into stream + whence - whence argument a la lseek + +=cut +*/ + +static +off_t +buffer_seek(io_glue *ig, off_t offset, int whence) { + io_ex_buffer *ieb = ig->exdata; + off_t reqpos = offset + + (whence == SEEK_CUR)*ieb->cpos + + (whence == SEEK_END)*ig->source.buffer.len; + + if (reqpos > ig->source.buffer.len) { + mm_log((1, "seeking out of readable range\n")); + return (off_t)-1; + } + + ieb->cpos = reqpos; + IOL_DEB( printf("buffer_seek(ig %p, offset %ld, whence %d)\n", ig, (long) offset, whence) ); + + return reqpos; + /* FIXME: How about implementing this offset handling stuff? */ +} + + + + + /* * Callbacks for sources that are a chain of variable sized buffers */ @@ -257,10 +383,13 @@ frees all resources used by a buffer chain. void io_destroy_bufchain(io_ex_bchain *ieb) { - io_blink *cp = ieb->head; + io_blink *cp; + mm_log((1, "io_destroy_bufchain(ieb %p)\n", ieb)); + cp = ieb->head; + while(cp) { io_blink *t = cp->next; - free(cp); + myfree(cp); cp = t; } } @@ -633,15 +762,17 @@ Sets an io_object for reading from a buffer source */ void -io_obj_setp_buffer(io_obj *io, void *p, size_t len) { - io->buffer.type = BUFFER; - io->buffer.c = (char*) p; - io->buffer.len = len; +io_obj_setp_buffer(io_obj *io, char *p, size_t len, closebufp closecb, void *closedata) { + io->buffer.type = BUFFER; + io->buffer.data = p; + io->buffer.len = len; + io->buffer.closecb = closecb; + io->buffer.closedata = closedata; } /* -=item io_obj_setp_cbuf(io, p) +=item io_obj_setp_buchain(io) Sets an io_object for reading/writing from a buffer source @@ -721,7 +852,6 @@ io_glue_commit_types(io_glue *ig) { } break; case CBSEEK: - default: { io_ex_rseek *ier = mymalloc(sizeof(io_ex_rseek)); @@ -734,6 +864,21 @@ io_glue_commit_types(io_glue *ig) { ig->seekcb = realseek_seek; ig->closecb = realseek_close; } + break; + case BUFFER: + { + io_ex_buffer *ieb = mymalloc(sizeof(io_ex_buffer)); + ieb->offset = 0; + ieb->cpos = 0; + + ig->exdata = ieb; + ig->readcb = buffer_read; + ig->writecb = buffer_write; + ig->seekcb = buffer_seek; + ig->closecb = buffer_close; + } + break; + default: } } @@ -776,7 +921,9 @@ be written to and read from later (like a pseudo file). io_glue * io_new_bufchain() { - io_glue *ig = mymalloc(sizeof(io_glue)); + io_glue *ig; + mm_log((1, "io_new_bufchain()\n")); + ig = mymalloc(sizeof(io_glue)); io_obj_setp_bufchain(&ig->source); return ig; } @@ -785,7 +932,27 @@ io_new_bufchain() { +/* +=item io_new_buffer(data, len) + +Returns a new io_glue object that has the source defined as reading +from specified buffer. Note that the buffer is not copied. + + data - buffer to read from + len - length of buffer +=cut +*/ + +io_glue * +io_new_buffer(char *data, size_t len, closebufp closecb, void *closedata) { + io_glue *ig; + mm_log((1, "io_new_buffer(data %p, len %d, closecb %p, closedata %p)\n", data, len, closecb, closedata)); + ig = mymalloc(sizeof(io_glue)); + memset(ig, 0, sizeof(*ig)); + io_obj_setp_buffer(&ig->source, data, len, closecb, closedata); + return ig; +} /* @@ -802,13 +969,16 @@ data from the io_glue callbacks hasn't been done yet. io_glue * io_new_fd(int fd) { - io_glue *ig = mymalloc(sizeof(io_glue)); + io_glue *ig; + mm_log((1, "io_new_fd(fd %d)\n", fd)); + ig = mymalloc(sizeof(io_glue)); memset(ig, 0, sizeof(*ig)); #ifdef _MSC_VER io_obj_setp_cb(&ig->source, (void*)fd, _read, _write, _lseek); #else io_obj_setp_cb(&ig->source, (void*)fd, read, write, lseek); #endif + mm_log((1, "(%p) <- io_new_fd\n", ig)); return ig; } @@ -877,6 +1047,7 @@ io_glue_DESTROY(io_glue *ig) { { io_ex_bchain *ieb = ig->exdata; io_destroy_bufchain(ieb); + myfree(ieb); } break; case CBSEEK: @@ -885,7 +1056,17 @@ io_glue_DESTROY(io_glue *ig) { io_ex_rseek *ier = ig->exdata; myfree(ier); } - + break; + case BUFFER: + { + io_ex_buffer *ieb = ig->exdata; + if (ig->source.buffer.closecb) { + mm_log((1,"calling close callback %p for io_buffer\n", ig->source.buffer.closecb)); + ig->source.buffer.closecb(ig->source.buffer.closedata); + } + myfree(ieb); + } + break; } myfree(ig); } diff --git a/iolayer.h b/iolayer.h index f5a0abfa..a17d0cec 100644 --- a/iolayer.h +++ b/iolayer.h @@ -42,6 +42,7 @@ typedef off_t (*seekp) (struct _io_glue *ig, off_t offset, int whence); typedef void (*closep)(struct _io_glue *ig); typedef ssize_t(*sizep) (struct _io_glue *ig); +typedef void (*closebufp)(void *p); /* Callbacks we get */ @@ -91,6 +92,12 @@ typedef struct { off_t gpos; /* Global position in stream */ } io_ex_bchain; +typedef struct { + off_t offset; /* Offset of the source - not used */ + off_t cpos; /* Offset within the current */ +} io_ex_buffer; + + /* Structures to describe data sources */ @@ -102,8 +109,10 @@ typedef struct { typedef struct { io_type type; /* Must be first parameter */ char *name; /* Data source name */ - char *c; + char *data; size_t len; + closebufp closecb; /* free memory mapped segment or decrement refcount */ + void *closedata; } io_buffer; typedef struct { @@ -133,7 +142,7 @@ typedef struct _io_glue { sizep sizecb; } io_glue; -void io_obj_setp_buffer (io_obj *io, void *p, size_t len); +void io_obj_setp_buffer(io_obj *io, char *p, size_t len, closebufp closecb, void *closedata); void io_obj_setp_cb (io_obj *io, void *p, readl readcb, writel writecb, seekl seekcb); void io_glue_commit_types(io_glue *ig); void io_glue_gettypes (io_glue *ig, int reqmeth); @@ -142,6 +151,7 @@ void io_glue_gettypes (io_glue *ig, int reqmeth); /* XS functions */ io_glue *io_new_fd(int fd); io_glue *io_new_bufchain(void); +io_glue *io_new_buffer(char *data, size_t len, closebufp closecb, void *closedata); size_t io_slurp(io_glue *ig, unsigned char **c); void io_glue_DESTROY(io_glue *ig); diff --git a/t/t07iolayer.t b/t/t07iolayer.t new file mode 100644 index 00000000..0add76eb --- /dev/null +++ b/t/t07iolayer.t @@ -0,0 +1,67 @@ +BEGIN { $|=1; print "1..7\n"; } +END { print "not ok 1\n" unless $loaded; }; +use Imager qw(:all); +++$loaded; +print "ok 1\n"; +init_log("testout/t07iolayer.log", 1); + + +undef($/); +# start by testing io buffer + +$data="P2\n2 2\n255\n 255 0\n0 255\n"; +$IO = Imager::io_new_buffer($data); +$im = Imager::i_readpnm_wiol($IO, -1); + +print "ok 2\n"; + + +open(FH, ">testout/t07.ppm") or die $!; +binmode(FH); +$fd = fileno(FH); +$IO2 = Imager::io_new_fd( $fd ); +Imager::i_writeppm_wiol($im, $IO2); +close(FH); +undef($im); + + + +open(FH, "; +close(FH); +$IO3 = Imager::io_new_buffer($data); +#undef($data); +$im = Imager::i_readpnm_wiol($IO3, -1); + +print "ok 3\n"; + + +open(FH, "tags == NULL) { int alloc = 10; - tags->tags = malloc(sizeof(i_img_tag) * alloc); + tags->tags = mymalloc(sizeof(i_img_tag) * alloc); if (!tags->tags) return 0; tags->alloc = alloc; @@ -115,15 +115,15 @@ int i_tags_add(i_img_tags *tags, char *name, int code, char *data, int size, tags->alloc = newalloc; } if (name) { - work.name = malloc(strlen(name)+1); + work.name = mymalloc(strlen(name)+1); if (!work.name) return 0; strcpy(work.name, name); } if (data) { - work.data = malloc(size+1); + work.data = mymalloc(size+1); if (!work.data) { - if (work.name) free(work.name); + if (work.name) myfree(work.name); return 0; } memcpy(work.data, data, size); @@ -142,11 +142,11 @@ void i_tags_destroy(i_img_tags *tags) { int i; for (i = 0; i < tags->count; ++i) { if (tags->tags[i].name) - free(tags->tags[i].name); + myfree(tags->tags[i].name); if (tags->tags[i].data) - free(tags->tags[i].data); + myfree(tags->tags[i].data); } - free(tags->tags); + myfree(tags->tags); } } @@ -182,9 +182,9 @@ int i_tags_delete(i_img_tags *tags, int entry) { memmove(tags->tags+entry, tags->tags+entry+1, tags->count-entry-1); if (old.name) - free(old.name); + myfree(old.name); if (old.data) - free(old.data); + myfree(old.data); --tags->count; return 1; } diff --git a/tga.c b/tga.c index 2f47c410..46f39003 100644 --- a/tga.c +++ b/tga.c @@ -689,7 +689,7 @@ i_readtga_wiol(io_glue *ig, int length) { if (idstring) { i_tags_add(&img->tags, "tga_idstring", 0, idstring, header.idlength, 0); - free(idstring); + myfree(idstring); } if (mapped && -- 2.30.2