From: Daniel Elstner Date: Sun, 20 Sep 2015 18:32:03 +0000 (+0200) Subject: srzip: Avoid low-level FD-based I/O X-Git-Tag: libsigrok-0.4.0~241 X-Git-Url: https://sigrok.org/gitaction?a=commitdiff_plain;h=98654c99daf85bd2a81a9c87f517e800c31e34f2;p=libsigrok.git srzip: Avoid low-level FD-based I/O Use in-memory buffers instead of temporary files. This avoids the need for low-level I/O on the FD returned by g_mkstemp(). Refactor the code accordingly. Also plug a number of leaks and tighten the error checking. --- diff --git a/src/libsigrok-internal.h b/src/libsigrok-internal.h index c8940c89..275deac4 100644 --- a/src/libsigrok-internal.h +++ b/src/libsigrok-internal.h @@ -34,6 +34,9 @@ #include #endif +struct zip; +struct zip_stat; + /** * @file * @@ -730,6 +733,11 @@ SR_PRIV int sr_packet_copy(const struct sr_datafeed_packet *packet, struct sr_datafeed_packet **copy); SR_PRIV void sr_packet_free(struct sr_datafeed_packet *packet); +/*--- session_file.c --------------------------------------------------------*/ + +SR_PRIV GKeyFile *sr_sessionfile_read_metadata(struct zip *archive, + const struct zip_stat *entry); + /*--- analog.c --------------------------------------------------------------*/ SR_PRIV int sr_analog_init(struct sr_datafeed_analog2 *analog, diff --git a/src/output/srzip.c b/src/output/srzip.c index d80a7a18..c562e604 100644 --- a/src/output/srzip.c +++ b/src/output/srzip.c @@ -19,7 +19,6 @@ #include #include -#include #include #include #include @@ -42,7 +41,7 @@ static int init(struct sr_output *o, GHashTable *options) (void)options; - if (strlen(o->filename) == 0) { + if (!o->filename || o->filename[0] == '\0') { sr_info("srzip output module requires a file name, cannot save."); return SR_ERR_ARG; } @@ -57,79 +56,84 @@ static int init(struct sr_output *o, GHashTable *options) static int zip_create(const struct sr_output *o) { struct out_context *outc; - struct sr_channel *ch; - FILE *meta; struct zip *zipfile; struct zip_source *versrc, *metasrc; + struct sr_channel *ch; GVariant *gvar; + GKeyFile *meta; GSList *l; - int tmpfile, ret; - char version[1], metafile[32], *s; + const char *devgroup; + char *s, *metabuf; + gsize metalen; outc = o->priv; - if (outc->samplerate == 0) { - if (sr_config_get(o->sdi->driver, o->sdi, NULL, SR_CONF_SAMPLERATE, - &gvar) == SR_OK) { - outc->samplerate = g_variant_get_uint64(gvar); - g_variant_unref(gvar); - } + + if (outc->samplerate == 0 && sr_config_get(o->sdi->driver, o->sdi, NULL, + SR_CONF_SAMPLERATE, &gvar) == SR_OK) { + outc->samplerate = g_variant_get_uint64(gvar); + g_variant_unref(gvar); } /* Quietly delete it first, libzip wants replace ops otherwise. */ - unlink(outc->filename); - if (!(zipfile = zip_open(outc->filename, ZIP_CREATE, &ret))) + g_unlink(outc->filename); + zipfile = zip_open(outc->filename, ZIP_CREATE, NULL); + if (!zipfile) return SR_ERR; /* "version" */ - version[0] = '2'; - if (!(versrc = zip_source_buffer(zipfile, version, 1, 0))) - return SR_ERR; - if (zip_add(zipfile, "version", versrc) == -1) { - sr_info("Error saving version into zipfile: %s.", + versrc = zip_source_buffer(zipfile, "2", 1, FALSE); + if (zip_file_add(zipfile, "version", versrc, 0) < 0) { + sr_err("Error saving version into zipfile: %s", zip_strerror(zipfile)); + zip_source_free(versrc); + zip_discard(zipfile); return SR_ERR; } /* init "metadata" */ - strcpy(metafile, "sigrok-meta-XXXXXX"); - if ((tmpfile = g_mkstemp(metafile)) == -1) - return SR_ERR; - close(tmpfile); - meta = g_fopen(metafile, "wb"); - fprintf(meta, "[global]\n"); - fprintf(meta, "sigrok version = %s\n", SR_PACKAGE_VERSION_STRING); - fprintf(meta, "[device 1]\ncapturefile = logic-1\n"); - fprintf(meta, "total probes = %d\n", g_slist_length(o->sdi->channels)); + meta = g_key_file_new(); + + g_key_file_set_string(meta, "global", "sigrok version", + SR_PACKAGE_VERSION_STRING); + + devgroup = "device 1"; + g_key_file_set_string(meta, devgroup, "capturefile", "logic-1"); + + g_key_file_set_integer(meta, devgroup, "total probes", + g_slist_length(o->sdi->channels)); + s = sr_samplerate_string(outc->samplerate); - fprintf(meta, "samplerate = %s\n", s); + g_key_file_set_string(meta, devgroup, "samplerate", s); g_free(s); for (l = o->sdi->channels; l; l = l->next) { ch = l->data; - if (ch->type != SR_CHANNEL_LOGIC) - continue; - if (!ch->enabled) - continue; - fprintf(meta, "probe%d = %s\n", ch->index + 1, ch->name); + if (ch->enabled && ch->type == SR_CHANNEL_LOGIC) { + s = g_strdup_printf("probe%d", ch->index + 1); + g_key_file_set_string(meta, devgroup, s, ch->name); + g_free(s); + } } - fclose(meta); + metabuf = g_key_file_to_data(meta, &metalen, NULL); + g_key_file_free(meta); - if (!(metasrc = zip_source_file(zipfile, metafile, 0, -1))) { - unlink(metafile); - return SR_ERR; - } - if (zip_add(zipfile, "metadata", metasrc) == -1) { - unlink(metafile); + metasrc = zip_source_buffer(zipfile, metabuf, metalen, FALSE); + if (zip_file_add(zipfile, "metadata", metasrc, 0) < 0) { + sr_err("Error saving metadata into zipfile: %s", + zip_strerror(zipfile)); + zip_source_free(metasrc); + zip_discard(zipfile); + g_free(metabuf); return SR_ERR; } - if ((ret = zip_close(zipfile)) == -1) { - sr_info("Error saving zipfile: %s.", zip_strerror(zipfile)); - unlink(metafile); + if (zip_close(zipfile) < 0) { + sr_err("Error saving zipfile: %s", zip_strerror(zipfile)); + zip_discard(zipfile); + g_free(metabuf); return SR_ERR; } - - unlink(metafile); + g_free(metabuf); return SR_OK; } @@ -140,72 +144,62 @@ static int zip_append(const struct sr_output *o, unsigned char *buf, struct out_context *outc; struct zip *archive; struct zip_source *logicsrc; - zip_int64_t num_files; - struct zip_file *zf; + int64_t i, num_files; struct zip_stat zs; struct zip_source *metasrc; GKeyFile *kf; GError *error; - gsize len; - int chunk_num, next_chunk_num, tmpfile, ret, i; + uint64_t chunk_num; const char *entry_name; - char *metafile, tmpname[32], chunkname[16]; + char *metabuf; + gsize metalen; + char *chunkname; + unsigned int next_chunk_num; outc = o->priv; - if (!(archive = zip_open(outc->filename, 0, &ret))) + if (!(archive = zip_open(outc->filename, 0, NULL))) return SR_ERR; - if (zip_stat(archive, "metadata", 0, &zs) == -1) + if (zip_stat(archive, "metadata", 0, &zs) < 0) { + sr_err("Failed to open metadata: %s", zip_strerror(archive)); + zip_discard(archive); return SR_ERR; - - metafile = g_malloc(zs.size); - zf = zip_fopen_index(archive, zs.index, 0); - zip_fread(zf, metafile, zs.size); - zip_fclose(zf); - + } + kf = sr_sessionfile_read_metadata(archive, &zs); + if (!kf) { + zip_discard(archive); + return SR_ERR_DATA; + } /* * If the file was only initialized but doesn't yet have any * data it in, it won't have a unitsize field in metadata yet. */ error = NULL; - kf = g_key_file_new(); - if (!g_key_file_load_from_data(kf, metafile, zs.size, 0, &error)) { - sr_err("Failed to parse metadata: %s.", error->message); - return SR_ERR; - } - g_free(metafile); - tmpname[0] = '\0'; + metabuf = NULL; if (!g_key_file_has_key(kf, "device 1", "unitsize", &error)) { if (error && error->code != G_KEY_FILE_ERROR_KEY_NOT_FOUND) { - sr_err("Failed to check unitsize key: %s", error ? error->message : "?"); + sr_err("Failed to check unitsize key: %s", error->message); + g_error_free(error); + g_key_file_free(kf); + zip_discard(archive); return SR_ERR; } + g_clear_error(&error); + /* Add unitsize field. */ g_key_file_set_integer(kf, "device 1", "unitsize", unitsize); - metafile = g_key_file_to_data(kf, &len, &error); - strcpy(tmpname, "sigrok-meta-XXXXXX"); - if ((tmpfile = g_mkstemp(tmpname)) == -1) - return SR_ERR; - if (write(tmpfile, metafile, len) < 0) { - sr_dbg("Failed to create new metadata: %s", g_strerror(errno)); - g_free(metafile); - unlink(tmpname); - return SR_ERR; - } - close(tmpfile); - if (!(metasrc = zip_source_file(archive, tmpname, 0, -1))) { - sr_err("Failed to create zip source for metadata."); - g_free(metafile); - unlink(tmpname); + metabuf = g_key_file_to_data(kf, &metalen, NULL); + metasrc = zip_source_buffer(archive, metabuf, metalen, FALSE); + + if (zip_replace(archive, zs.index, metasrc) < 0) { + sr_err("Failed to replace metadata: %s", + zip_strerror(archive)); + g_key_file_free(kf); + zip_source_free(metasrc); + zip_discard(archive); + g_free(metabuf); return SR_ERR; } - if (zip_replace(archive, zs.index, metasrc) == -1) { - sr_err("Failed to replace metadata file."); - g_free(metafile); - unlink(tmpname); - return SR_ERR; - } - g_free(metafile); } g_key_file_free(kf); @@ -213,39 +207,50 @@ static int zip_append(const struct sr_output *o, unsigned char *buf, num_files = zip_get_num_entries(archive, 0); for (i = 0; i < num_files; i++) { entry_name = zip_get_name(archive, i, 0); - if (strncmp(entry_name, "logic-1", 7)) + if (!entry_name || strncmp(entry_name, "logic-1", 7) != 0) continue; - if (strlen(entry_name) == 7) { + if (entry_name[7] == '\0') { /* This file has no extra chunks, just a single "logic-1". * Rename it to "logic-1-1" * and continue with chunk 2. */ - if (zip_rename(archive, i, "logic-1-1") == -1) { - sr_err("Failed to rename 'logic-1' to 'logic-1-1'."); - unlink(tmpname); + if (zip_rename(archive, i, "logic-1-1") < 0) { + sr_err("Failed to rename 'logic-1' to 'logic-1-1': %s", + zip_strerror(archive)); + zip_discard(archive); + g_free(metabuf); return SR_ERR; } next_chunk_num = 2; break; - } else if (strlen(entry_name) > 8 && entry_name[7] == '-') { - chunk_num = strtoull(entry_name + 8, NULL, 10); - if (chunk_num >= next_chunk_num) + } else if (entry_name[7] == '-') { + chunk_num = g_ascii_strtoull(entry_name + 8, NULL, 10); + if (chunk_num < G_MAXINT && chunk_num >= next_chunk_num) next_chunk_num = chunk_num + 1; } } - snprintf(chunkname, 15, "logic-1-%d", next_chunk_num); - if (!(logicsrc = zip_source_buffer(archive, buf, length, FALSE))) { - unlink(tmpname); - return SR_ERR; + + if (length % unitsize != 0) { + sr_warn("Chunk size %d not a multiple of the" + " unit size %d.", length, unitsize); } - if (zip_add(archive, chunkname, logicsrc) == -1) { - unlink(tmpname); + logicsrc = zip_source_buffer(archive, buf, length, FALSE); + chunkname = g_strdup_printf("logic-1-%u", next_chunk_num); + i = zip_add(archive, chunkname, logicsrc); + g_free(chunkname); + if (i < 0) { + sr_err("Failed to add chunk 'logic-1-%u': %s", + next_chunk_num, zip_strerror(archive)); + zip_source_free(logicsrc); + zip_discard(archive); + g_free(metabuf); return SR_ERR; } - if ((ret = zip_close(archive)) == -1) { - sr_info("error saving session file: %s", zip_strerror(archive)); - unlink(tmpname); + if (zip_close(archive) < 0) { + sr_err("Error saving session file: %s", zip_strerror(archive)); + zip_discard(archive); + g_free(metabuf); return SR_ERR; } - unlink(tmpname); + g_free(metabuf); return SR_OK; }