From faac186220c7a01279e35fa87fd7614bd2b6f68b Mon Sep 17 00:00:00 2001 From: Thomas Schmitt Date: Mon, 9 Oct 2006 12:49:08 +0000 Subject: [PATCH] Got rid of assert() in mmc.c by soft means --- trunk/cdrskin/cdrskin_timestamp.h | 2 +- trunk/libburn/asserts.txt | 77 +++++++++++++++++++++++++------ trunk/libburn/mmc.c | 74 ++++++++++++++++++++++++----- trunk/libburn/mmc.h | 9 +++- trunk/libburn/read.c | 9 +++- trunk/libburn/sector.c | 11 ++++- trunk/libburn/transport.h | 11 ++++- 7 files changed, 161 insertions(+), 32 deletions(-) diff --git a/trunk/cdrskin/cdrskin_timestamp.h b/trunk/cdrskin/cdrskin_timestamp.h index 596ff866..986dfdcd 100644 --- a/trunk/cdrskin/cdrskin_timestamp.h +++ b/trunk/cdrskin/cdrskin_timestamp.h @@ -1 +1 @@ -#define Cdrskin_timestamP "2006.10.09.083438" +#define Cdrskin_timestamP "2006.10.09.123518" diff --git a/trunk/libburn/asserts.txt b/trunk/libburn/asserts.txt index 2c4ca2f5..504fa967 100644 --- a/trunk/libburn/asserts.txt +++ b/trunk/libburn/asserts.txt @@ -229,60 +229,107 @@ ts A61007 ------------------------------------------------------------------------------ - 17) libburn/mmc.c: assert(o->drive == d); +++ 17) libburn/mmc.c: assert(o->drive == d); mmc_close_disc(): alias: struct burn_drive.close_disc() Parameters struct burn_drive and struct burn_write_opts do not match Called by -nobody- ? -=> Disable unused function ? +( => Disable unused function ? ) +=> removed redundant parameter struct burn_drive + +ts A61009 ------------------------------------------------------------------------------ - 18) libburn/mmc.c: assert(o->drive == d); +++ 18) libburn/mmc.c: assert(o->drive == d); mmc_close_session(): Same as 17) alias: struct burn_drive.close_session() Called by -nobody- ? -=> Disable unused function ? +( => Disable unused function ? ) +=> removed redundant parameter struct burn_drive + +ts A61009 ------------------------------------------------------------------------------ - 19) libburn/mmc.c: assert(buf->bytes >= buf->sectors); /* can be == at 0... */ +++ 19) libburn/mmc.c: assert(buf->bytes >= buf->sectors); /* can be == at 0... */ mmc_write_12(): - Unclear what .bytes and .sectors mean in struct buffer - Called by -nobody- ? -=> Disable unused function ? +=> problems with filling the write buffer have to be handled by callers +=> delete assert + +ts A61009 ------------------------------------------------------------------------------ - 20) libburn/mmc.c: assert(buf->bytes >= buf->sectors); /* can be == at 0... */ +++ 20) libburn/mmc.c: assert(buf->bytes >= buf->sectors); /* can be == at 0... */ mmc_write(): - Unclear what .bytes and .sectors mean in struct buffer - -alias: struct burn_drive.write() +libburn/mmc.c: c.page->sectors = errorblock - start + 1; +mmc_read_sectors() by toc_find_modes() by mmc_read_toc() alias drive.read_toc() +by burn_drive_grab() +This seems to be unrelated to mmc_write(). + +libburn/sector.c: out->sectors++; +get_sector() +Seems to hand out sector start pointer in opts->drive->buffer +and to count reservation transactions as well as reserved bytes. +Ensures out->bytes >= out->sectors + + +libburn/mmc.c: c.page->bytes = s->count * 8; +mmc_send_cue_sheet() +Does not use mmc_write() but directly (sg_)issue_command() + +libburn/sector.c: out->bytes += seclen; +get_sector() +See above +Ensures out->bytes >= out->sectors + +libburn/spc.c: c.page->bytes = 8 + 2 + d->mdata->retry_page_length; +spc_select_error_params() +Does not use mmc_write() but directly (sg_)issue_command() + +libburn/spc.c: c.page->bytes = 8 + 2 + d->mdata->write_page_length; +spc_select_error_params() +Does not use mmc_write() but directly (sg_)issue_command() + +libburn/spc.c: c.page->bytes = 8 + 2 + 0x32; +spc_probe_write_modes() +Does not use mmc_write() but directly (sg_)issue_command() + +alias struct burn_drive.write() Called by static get_sector, by many Called by burn_write_flush Called by burn_write_track -=> ??? +=> problems with filling the write buffer have to be handled by callers +=> delete assert + +ts A61009 ------------------------------------------------------------------------------ - 21) libburn/mmc.c: assert(((dlen - 2) % 11) == 0); +++ 21) libburn/mmc.c: assert(((dlen - 2) % 11) == 0); mmc_read_toc(): - Is defunct - => :) +ts A61009 + ------------------------------------------------------------------------------ - 22) libburn/mmc.c: assert(len >= 0); +++ 22) libburn/mmc.c: assert(len >= 0); mmc_read_sectors(): Catches a bad parameter @@ -294,9 +341,11 @@ Called by toc_find_modes(), problem cannot occur: mem.sectors = 1; (=> in burn_disc_read() check page.sectors before d->read_sectors() ) => :) +ts A61009 + ------------------------------------------------------------------------------ - 23) libburn/mmc.c: assert(d->busy); +++ 23) libburn/mmc.c: assert(d->busy); mmc_read_sectors(): Catches use of a drive that is not marked as busy @@ -307,6 +356,8 @@ Called by toc_find_modes(), does the same assert. To be solved there. : Severe Libburn Error => :) +ts A61009 + ------------------------------------------------------------------------------ ++ 24) libburn/options.c: assert(0); @@ -563,7 +614,7 @@ ts A61008 ------------------------------------------------------------------------------ - 43) libburn/structure.c: assert(s->track != NULL); +++ 43) libburn/structure.c: assert(s->track != NULL); API burn_session_remove_track() An application supplied pointer is NULL diff --git a/trunk/libburn/mmc.c b/trunk/libburn/mmc.c index 17705e2c..b4a6aded 100644 --- a/trunk/libburn/mmc.c +++ b/trunk/libburn/mmc.c @@ -1,6 +1,8 @@ /* -*- indent-tabs-mode: t; tab-width: 8; c-basic-offset: 8; -*- */ -#include +/* ts A61009 */ +/* #include */ + #include #include #include @@ -18,6 +20,12 @@ #include "structure.h" #include "options.h" +/* ts A61005 */ +#include "libdax_msgs.h" +extern struct libdax_msgs *libdax_messenger; + + + static unsigned char MMC_GET_TOC[] = { 0x43, 2, 2, 0, 0, 0, 0, 16, 0, 0 }; static unsigned char MMC_GET_ATIP[] = { 0x43, 2, 4, 0, 0, 0, 0, 16, 0, 0 }; static unsigned char MMC_GET_DISC_INFO[] = @@ -99,19 +107,43 @@ int mmc_get_nwa(struct burn_drive *d) + (data[14] << 8) + data[15]; } -void mmc_close_disc(struct burn_drive *d, struct burn_write_opts *o) +/* ts A61009 : function is obviously unused. */ +/* void mmc_close_disc(struct burn_drive *d, struct burn_write_opts *o) */ +void mmc_close_disc(struct burn_write_opts *o) { + struct burn_drive *d; + mmc_function_spy("mmc_close_disc"); - assert(o->drive == d); + + libdax_msgs_submit(libdax_messenger, -1, 0x00000002, + LIBDAX_MSGS_SEV_DEBUG, LIBDAX_MSGS_PRIO_ZERO, + "HOW THAT ? mmc_close_disc() was called", 0, 0); + + /* ts A61009 : made impossible by removing redundant parameter d */ + /* a ssert(o->drive == d); */ + d = o->drive; + o->multi = 0; spc_select_write_params(d, o); mmc_close(d, 1, 0); } -void mmc_close_session(struct burn_drive *d, struct burn_write_opts *o) +/* ts A61009 : function is obviously unused. */ +/* void mmc_close_session(struct burn_drive *d, struct burn_write_opts *o) */ +void mmc_close_session(struct burn_write_opts *o) { + struct burn_drive *d; + mmc_function_spy("mmc_close_session"); - assert(o->drive == d); + + libdax_msgs_submit(libdax_messenger, -1, 0x00000002, + LIBDAX_MSGS_SEV_DEBUG, LIBDAX_MSGS_PRIO_ZERO, + "HOW THAT ? mmc_close_session() was called", 0, 0); + + /* ts A61009 : made impossible by removing redundant parameter d */ + /* a ssert(o->drive == d); */ + d = o->drive; + o->multi = 3; spc_select_write_params(d, o); mmc_close(d, 1, 0); @@ -121,7 +153,12 @@ void mmc_close(struct burn_drive *d, int session, int track) { struct command c; - mmc_function_spy("mmc_close_session"); + mmc_function_spy("mmc_close"); + + libdax_msgs_submit(libdax_messenger, -1, 0x00000002, + LIBDAX_MSGS_SEV_DEBUG, LIBDAX_MSGS_PRIO_ZERO, + "HOW THAT ? mmc_close() was called", 0, 0); + c.retry = 1; c.oplen = sizeof(MMC_CLOSE); memcpy(c.opcode, MMC_CLOSE, sizeof(MMC_CLOSE)); @@ -154,6 +191,7 @@ void mmc_get_event(struct burn_drive *d) c.page->data[5], c.page->data[6], c.page->data[7]); } + void mmc_write_12(struct burn_drive *d, int start, struct buffer *buf) { struct command c; @@ -161,7 +199,10 @@ void mmc_write_12(struct burn_drive *d, int start, struct buffer *buf) mmc_function_spy("mmc_write_12"); len = buf->sectors; - assert(buf->bytes >= buf->sectors); /* can be == at 0... */ + + /* ts A61009 */ + /* a ssert(buf->bytes >= buf->sectors);*/ /* can be == at 0... */ + burn_print(100, "trying to write %d at %d\n", len, start); memcpy(c.opcode, MMC_WRITE_12, sizeof(MMC_WRITE_12)); c.retry = 1; @@ -195,7 +236,10 @@ int mmc_write(struct burn_drive *d, int start, struct buffer *buf) return BE_CANCELLED; len = buf->sectors; - assert(buf->bytes >= buf->sectors); /* can be == at 0... */ + + /* ts A61009 : buffer fill problems are to be handled by caller */ + /* a ssert(buf->bytes >= buf->sectors);*/ /* can be == at 0... */ + burn_print(100, "trying to write %d at %d\n", len, start); memcpy(c.opcode, MMC_WRITE_10, sizeof(MMC_WRITE_10)); c.retry = 1; @@ -251,7 +295,8 @@ void mmc_read_toc(struct burn_drive *d) /* some drives fail this check. - assert(((dlen - 2) % 11) == 0); + ts A61007 : if re-enabled then not via Assert. + a ssert(((dlen - 2) % 11) == 0); */ d->toc_entry = malloc(d->toc_entries * sizeof(struct burn_toc_entry)); tdata = c.page->data + 4; @@ -382,9 +427,14 @@ void mmc_read_sectors(struct burn_drive *d, struct command c; mmc_function_spy("mmc_read_sectors"); - assert(len >= 0); -/* if the drive isn't busy, why the hell are we here? */ - assert(d->busy); + + /* ts A61009 : to be ensured by callers */ + /* a ssert(len >= 0); */ + +/* if the drive isn't busy, why the hell are we here? */ + /* ts A61006 : i second that question */ + /* a ssert(d->busy); */ + burn_print(12, "reading %d from %d\n", len, start); memcpy(c.opcode, MMC_READ_CD, sizeof(MMC_READ_CD)); c.retry = 1; diff --git a/trunk/libburn/mmc.h b/trunk/libburn/mmc.h index 95275b7c..6a0044e6 100644 --- a/trunk/libburn/mmc.h +++ b/trunk/libburn/mmc.h @@ -12,8 +12,13 @@ struct cue_sheet; /* MMC commands */ void mmc_read(struct burn_drive *); -void mmc_close_session(struct burn_drive *, struct burn_write_opts *); -void mmc_close_disc(struct burn_drive *, struct burn_write_opts *); + +/* ts A61009 : removed redundant parameter d in favor of o->drive */ +/* void mmc_close_session(struct burn_drive *, struct burn_write_opts *); */ +/* void mmc_close_disc(struct burn_drive *, struct burn_write_opts *); */ +void mmc_close_session(struct burn_write_opts *o); +void mmc_close_disc(struct burn_write_opts *o); + void mmc_close(struct burn_drive *, int session, int track); void mmc_get_event(struct burn_drive *); int mmc_write(struct burn_drive *, int start, struct buffer *buf); diff --git a/trunk/libburn/read.c b/trunk/libburn/read.c index e404ec24..4f27207a 100644 --- a/trunk/libburn/read.c +++ b/trunk/libburn/read.c @@ -47,6 +47,9 @@ void burn_disc_read(struct burn_drive *d, const struct burn_read_opts *o) a ssert(d->toc->valid); a ssert(o->datafd != -1); + /* moved up from spc_select_error_params alias d->send_parameters() */ + a ssert(d->mdata->valid); + /* XXX not sure this is a good idea. copy it? */ /* XXX also, we have duplicated data now, do we remove the fds from struct drive, or only store a subset of the _opts structs in drives */ @@ -56,6 +59,7 @@ drive, or only store a subset of the _opts structs in drives */ d->set_speed(d, speed, 0); d->params.retries = o->hardware_error_retries; + d->send_parameters(d, o); d->cancel = 0; @@ -133,7 +137,10 @@ drive, or only store a subset of the _opts structs in drives */ page.sectors = (finish < maxsects) ? finish : maxsects; printf("reading %d sectors from %d\n", page.sectors, drive_lba); - d->read_sectors(d, drive_lba, page.sectors, o, &page); + + /* >>> ts A61009 : ensure page.sectors >= 0 before calling */ + d->r ead_sectors(d, drive_lba, page.sectors, o, &page); + printf("Read %d\n", page.sectors); } #endif diff --git a/trunk/libburn/sector.c b/trunk/libburn/sector.c index a1ec9f1b..3b1319e9 100644 --- a/trunk/libburn/sector.c +++ b/trunk/libburn/sector.c @@ -148,6 +148,9 @@ static void get_bytes(struct burn_track *track, int count, unsigned char *data) return; memset(data + curr, 0, shortage); } + +/* ts A61009 : seems to hand out sector start pointer in opts->drive->buffer + and to count hand outs as well as reserved bytes */ static unsigned char *get_sector(struct burn_write_opts *opts, int inmode) { struct burn_drive *d = opts->drive; @@ -160,7 +163,13 @@ static unsigned char *get_sector(struct burn_write_opts *opts, int inmode) if (outmode == 0) outmode = inmode; - seclen = burn_sector_length(outmode) + burn_subcode_length(outmode); + /* ts A61009 : react on eventual failure of burn_sector_length() + (should not happen if API tested properly). + Ensures out->bytes >= out->sectors */ + seclen = burn_sector_length(outmode); + if (seclen <= 0) + return NULL; + seclen += burn_subcode_length(outmode); if (out->bytes + (seclen) >= BUFFER_SIZE) { int err; diff --git a/trunk/libburn/transport.h b/trunk/libburn/transport.h index 47ae092e..7b903179 100644 --- a/trunk/libburn/transport.h +++ b/trunk/libburn/transport.h @@ -161,9 +161,16 @@ struct burn_drive void (*sync_cache) (struct burn_drive *); int (*get_erase_progress) (struct burn_drive *); int (*get_nwa) (struct burn_drive *); - void (*close_disc) (struct burn_drive * d, struct burn_write_opts * o); - void (*close_session) (struct burn_drive * d, + + /* ts A61009 : removed d in favor of o->drive */ + /* void (*close_disc) (struct burn_drive * d, + struct burn_write_opts * o); + void (*close_session) (struct burn_drive * d, struct burn_write_opts * o); + */ + void (*close_disc) (struct burn_write_opts * o); + void (*close_session) ( struct burn_write_opts * o); + int (*test_unit_ready) (struct burn_drive * d); void (*probe_write_modes) (struct burn_drive * d); struct params params;