BLAHAJ
/
libsigrok
10
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

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.
This commit is contained in:
Daniel Elstner 2015-09-20 20:32:03 +02:00
parent 5e1fb33469
commit 98654c99da
2 changed files with 121 additions and 108 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
src/libsigrok-internal.h
View File

@ -34,6 +34,9 @@
#include <libserialport.h> #include <libserialport.h>
#endif #endif
struct zip;
struct zip_stat;
/** /**
* @file * @file
* *
@ -730,6 +733,11 @@ SR_PRIV int sr_packet_copy(const struct sr_datafeed_packet *packet,
struct sr_datafeed_packet **copy); struct sr_datafeed_packet **copy);
SR_PRIV void sr_packet_free(struct sr_datafeed_packet *packet); 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 --------------------------------------------------------------*/ /*--- analog.c --------------------------------------------------------------*/
SR_PRIV int sr_analog_init(struct sr_datafeed_analog2 *analog, SR_PRIV int sr_analog_init(struct sr_datafeed_analog2 *analog,

221
src/output/srzip.c
View File

@ -19,7 +19,6 @@
#include <config.h> #include <config.h>
#include <stdlib.h> #include <stdlib.h>
#include <unistd.h>
#include <string.h> #include <string.h>
#include <errno.h> #include <errno.h>
#include <glib.h> #include <glib.h>
@ -42,7 +41,7 @@ static int init(struct sr_output *o, GHashTable *options)
(void)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."); sr_info("srzip output module requires a file name, cannot save.");
return SR_ERR_ARG; 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) static int zip_create(const struct sr_output *o)
{ {
struct out_context *outc; struct out_context *outc;
struct sr_channel *ch;
FILE *meta;
struct zip *zipfile; struct zip *zipfile;
struct zip_source *versrc, *metasrc; struct zip_source *versrc, *metasrc;
struct sr_channel *ch;
GVariant *gvar; GVariant *gvar;
GKeyFile *meta;
GSList *l; GSList *l;
int tmpfile, ret; const char *devgroup;
char version[1], metafile[32], *s; char *s, *metabuf;
gsize metalen;
outc = o->priv; outc = o->priv;
if (outc->samplerate == 0) {
if (sr_config_get(o->sdi->driver, o->sdi, NULL, SR_CONF_SAMPLERATE, if (outc->samplerate == 0 && sr_config_get(o->sdi->driver, o->sdi, NULL,
&gvar) == SR_OK) { SR_CONF_SAMPLERATE, &gvar) == SR_OK) {
outc->samplerate = g_variant_get_uint64(gvar); outc->samplerate = g_variant_get_uint64(gvar);
g_variant_unref(gvar); g_variant_unref(gvar);
}
} }
/* Quietly delete it first, libzip wants replace ops otherwise. */ /* Quietly delete it first, libzip wants replace ops otherwise. */
unlink(outc->filename); g_unlink(outc->filename);
if (!(zipfile = zip_open(outc->filename, ZIP_CREATE, &ret))) zipfile = zip_open(outc->filename, ZIP_CREATE, NULL);
if (!zipfile)
return SR_ERR; return SR_ERR;
/* "version" */ /* "version" */
version[0] = '2'; versrc = zip_source_buffer(zipfile, "2", 1, FALSE);
if (!(versrc = zip_source_buffer(zipfile, version, 1, 0))) if (zip_file_add(zipfile, "version", versrc, 0) < 0) {
return SR_ERR; sr_err("Error saving version into zipfile: %s",
if (zip_add(zipfile, "version", versrc) == -1) {
sr_info("Error saving version into zipfile: %s.",
zip_strerror(zipfile)); zip_strerror(zipfile));
zip_source_free(versrc);
zip_discard(zipfile);
return SR_ERR; return SR_ERR;
} }
/* init "metadata" */ /* init "metadata" */
strcpy(metafile, "sigrok-meta-XXXXXX"); meta = g_key_file_new();
if ((tmpfile = g_mkstemp(metafile)) == -1)
return SR_ERR; g_key_file_set_string(meta, "global", "sigrok version",
close(tmpfile); SR_PACKAGE_VERSION_STRING);
meta = g_fopen(metafile, "wb");
fprintf(meta, "[global]\n"); devgroup = "device 1";
fprintf(meta, "sigrok version = %s\n", SR_PACKAGE_VERSION_STRING); g_key_file_set_string(meta, devgroup, "capturefile", "logic-1");
fprintf(meta, "[device 1]\ncapturefile = logic-1\n");
fprintf(meta, "total probes = %d\n", g_slist_length(o->sdi->channels)); g_key_file_set_integer(meta, devgroup, "total probes",
g_slist_length(o->sdi->channels));
s = sr_samplerate_string(outc->samplerate); s = sr_samplerate_string(outc->samplerate);
fprintf(meta, "samplerate = %s\n", s); g_key_file_set_string(meta, devgroup, "samplerate", s);
g_free(s); g_free(s);
for (l = o->sdi->channels; l; l = l->next) { for (l = o->sdi->channels; l; l = l->next) {
ch = l->data; ch = l->data;
if (ch->type != SR_CHANNEL_LOGIC) if (ch->enabled && ch->type == SR_CHANNEL_LOGIC) {
continue; s = g_strdup_printf("probe%d", ch->index + 1);
if (!ch->enabled) g_key_file_set_string(meta, devgroup, s, ch->name);
continue; g_free(s);
fprintf(meta, "probe%d = %s\n", ch->index + 1, ch->name); }
} }
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))) { metasrc = zip_source_buffer(zipfile, metabuf, metalen, FALSE);
unlink(metafile); if (zip_file_add(zipfile, "metadata", metasrc, 0) < 0) {
return SR_ERR; sr_err("Error saving metadata into zipfile: %s",
} zip_strerror(zipfile));
if (zip_add(zipfile, "metadata", metasrc) == -1) { zip_source_free(metasrc);
unlink(metafile); zip_discard(zipfile);
g_free(metabuf);
return SR_ERR; return SR_ERR;
} }
if ((ret = zip_close(zipfile)) == -1) { if (zip_close(zipfile) < 0) {
sr_info("Error saving zipfile: %s.", zip_strerror(zipfile)); sr_err("Error saving zipfile: %s", zip_strerror(zipfile));
unlink(metafile); zip_discard(zipfile);
g_free(metabuf);
return SR_ERR; return SR_ERR;
} }
g_free(metabuf);
unlink(metafile);
return SR_OK; return SR_OK;
} }
@ -140,72 +144,62 @@ static int zip_append(const struct sr_output *o, unsigned char *buf,
struct out_context *outc; struct out_context *outc;
struct zip *archive; struct zip *archive;
struct zip_source *logicsrc; struct zip_source *logicsrc;
zip_int64_t num_files; int64_t i, num_files;
struct zip_file *zf;
struct zip_stat zs; struct zip_stat zs;
struct zip_source *metasrc; struct zip_source *metasrc;
GKeyFile *kf; GKeyFile *kf;
GError *error; GError *error;
gsize len; uint64_t chunk_num;
int chunk_num, next_chunk_num, tmpfile, ret, i;
const char *entry_name; const char *entry_name;
char *metafile, tmpname[32], chunkname[16]; char *metabuf;
gsize metalen;
char *chunkname;
unsigned int next_chunk_num;
outc = o->priv; outc = o->priv;
if (!(archive = zip_open(outc->filename, 0, &ret))) if (!(archive = zip_open(outc->filename, 0, NULL)))
return SR_ERR; 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; return SR_ERR;
}
metafile = g_malloc(zs.size); kf = sr_sessionfile_read_metadata(archive, &zs);
zf = zip_fopen_index(archive, zs.index, 0); if (!kf) {
zip_fread(zf, metafile, zs.size); zip_discard(archive);
zip_fclose(zf); return SR_ERR_DATA;
}
/* /*
* If the file was only initialized but doesn't yet have any * 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. * data it in, it won't have a unitsize field in metadata yet.
*/ */
error = NULL; error = NULL;
kf = g_key_file_new(); metabuf = NULL;
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';
if (!g_key_file_has_key(kf, "device 1", "unitsize", &error)) { if (!g_key_file_has_key(kf, "device 1", "unitsize", &error)) {
if (error && error->code != G_KEY_FILE_ERROR_KEY_NOT_FOUND) { 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; return SR_ERR;
} }
g_clear_error(&error);
/* Add unitsize field. */ /* Add unitsize field. */
g_key_file_set_integer(kf, "device 1", "unitsize", unitsize); g_key_file_set_integer(kf, "device 1", "unitsize", unitsize);
metafile = g_key_file_to_data(kf, &len, &error); metabuf = g_key_file_to_data(kf, &metalen, NULL);
strcpy(tmpname, "sigrok-meta-XXXXXX"); metasrc = zip_source_buffer(archive, metabuf, metalen, FALSE);
if ((tmpfile = g_mkstemp(tmpname)) == -1)
return SR_ERR; if (zip_replace(archive, zs.index, metasrc) < 0) {
if (write(tmpfile, metafile, len) < 0) { sr_err("Failed to replace metadata: %s",
sr_dbg("Failed to create new metadata: %s", g_strerror(errno)); zip_strerror(archive));
g_free(metafile); g_key_file_free(kf);
unlink(tmpname); zip_source_free(metasrc);
zip_discard(archive);
g_free(metabuf);
return SR_ERR; 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);
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); 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); num_files = zip_get_num_entries(archive, 0);
for (i = 0; i < num_files; i++) { for (i = 0; i < num_files; i++) {
entry_name = zip_get_name(archive, i, 0); 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; continue;
if (strlen(entry_name) == 7) { if (entry_name[7] == '\0') {
/* This file has no extra chunks, just a single "logic-1". /* This file has no extra chunks, just a single "logic-1".
* Rename it to "logic-1-1" * and continue with chunk 2. */ * Rename it to "logic-1-1" * and continue with chunk 2. */
if (zip_rename(archive, i, "logic-1-1") == -1) { if (zip_rename(archive, i, "logic-1-1") < 0) {
sr_err("Failed to rename 'logic-1' to 'logic-1-1'."); sr_err("Failed to rename 'logic-1' to 'logic-1-1': %s",
unlink(tmpname); zip_strerror(archive));
zip_discard(archive);
g_free(metabuf);
return SR_ERR; return SR_ERR;
} }
next_chunk_num = 2; next_chunk_num = 2;
break; break;
} else if (strlen(entry_name) > 8 && entry_name[7] == '-') { } else if (entry_name[7] == '-') {
chunk_num = strtoull(entry_name + 8, NULL, 10); chunk_num = g_ascii_strtoull(entry_name + 8, NULL, 10);
if (chunk_num >= next_chunk_num) if (chunk_num < G_MAXINT && chunk_num >= next_chunk_num)
next_chunk_num = chunk_num + 1; next_chunk_num = chunk_num + 1;
} }
} }
snprintf(chunkname, 15, "logic-1-%d", next_chunk_num);
if (!(logicsrc = zip_source_buffer(archive, buf, length, FALSE))) { if (length % unitsize != 0) {
unlink(tmpname); sr_warn("Chunk size %d not a multiple of the"
" unit size %d.", length, unitsize);
}
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; return SR_ERR;
} }
if (zip_add(archive, chunkname, logicsrc) == -1) { if (zip_close(archive) < 0) {
unlink(tmpname); sr_err("Error saving session file: %s", zip_strerror(archive));
zip_discard(archive);
g_free(metabuf);
return SR_ERR; return SR_ERR;
} }
if ((ret = zip_close(archive)) == -1) { g_free(metabuf);
sr_info("error saving session file: %s", zip_strerror(archive));
unlink(tmpname);
return SR_ERR;
}
unlink(tmpname);
return SR_OK; return SR_OK;
} }