Issue #11: improve types in file i/o functions

Use open(2)/read(2)/write(2) instead of fopen(3)/fread(3)/fwrite(3) in
several places to avoid unnecessary buffering

Fix a bug in Rhizome HTTP add's handling of unconfigured manifest
template file

Improve some debug and error logging for file i/o
This commit is contained in:
Andrew Bettison 2013-10-11 16:06:12 +10:30
parent 49729cc768
commit 97cbebc91e
8 changed files with 49 additions and 46 deletions

View File

@ -1559,7 +1559,7 @@ int app_rhizome_append_manifest(const struct cli_parsed *parsed, struct cli_cont
return WHY("Out of manifests."); return WHY("Out of manifests.");
int ret=0; int ret=0;
if (rhizome_read_manifest_file(m, manifestpath, 0)) if (rhizome_read_manifest_file(m, manifestpath, 0) == -1)
ret=-1; ret=-1;
// TODO why doesn't read manifest file set finalised??? // TODO why doesn't read manifest file set finalised???
m->finalised=1; m->finalised=1;

8
net.c
View File

@ -77,11 +77,11 @@ ssize_t _write_all(int fd, const void *buf, size_t len, struct __sourceloc __whe
{ {
ssize_t written = write(fd, buf, len); ssize_t written = write(fd, buf, len);
if (written == -1) if (written == -1)
return WHYF_perror("write_all: write(%d,%p %s,%lu)", return WHYF_perror("write_all: write(%d,%p %s,%zu)",
fd, buf, alloca_toprint(30, buf, len), (unsigned long)len); fd, buf, alloca_toprint(30, buf, len), len);
if (written != len) if (written != len)
return WHYF_perror("write_all: write(%d,%p %s,%lu) returned %ld", return WHYF_perror("write_all: write(%d,%p %s,%zu) returned %zd",
fd, buf, alloca_toprint(30, buf, len), (unsigned long)len, (long)written); fd, buf, alloca_toprint(30, buf, len), len, (size_t)written);
return written; return written;
} }

View File

@ -288,7 +288,7 @@ int rhizome_write_manifest_file(rhizome_manifest *m, const char *filename, char
int rhizome_manifest_selfsign(rhizome_manifest *m); int rhizome_manifest_selfsign(rhizome_manifest *m);
int rhizome_drop_stored_file(const rhizome_filehash_t *hashp, int maximum_priority); int rhizome_drop_stored_file(const rhizome_filehash_t *hashp, int maximum_priority);
int rhizome_manifest_priority(sqlite_retry_state *retry, const rhizome_bid_t *bidp); int rhizome_manifest_priority(sqlite_retry_state *retry, const rhizome_bid_t *bidp);
int rhizome_read_manifest_file(rhizome_manifest *m, const char *filename, int bufferPAndSize); int rhizome_read_manifest_file(rhizome_manifest *m, const char *filename, size_t bufferPAndSize);
int rhizome_hash_file(rhizome_manifest *m, const char *path, rhizome_filehash_t *hash_out, uint64_t *size_out); int rhizome_hash_file(rhizome_manifest *m, const char *path, rhizome_filehash_t *hash_out, uint64_t *size_out);
char *rhizome_manifest_get(const rhizome_manifest *m, const char *var, char *out, int maxlen); char *rhizome_manifest_get(const rhizome_manifest *m, const char *var, char *out, int maxlen);
int64_t rhizome_manifest_get_ll(rhizome_manifest *m, const char *var); int64_t rhizome_manifest_get_ll(rhizome_manifest *m, const char *var);

View File

@ -85,16 +85,16 @@ int rhizome_manifest_verify(rhizome_manifest *m)
else return 0; else return 0;
} }
int read_whole_file(const char *filename, unsigned char *buffer, int buffer_size) ssize_t read_whole_file(const char *path, unsigned char *buffer, size_t buffer_size)
{ {
FILE *f = fopen(filename, "r"); int fd = open(path, O_RDONLY);
if (f == NULL) if (fd == -1)
return WHYF("Could not open file %s for reading", filename); return WHYF_perror("open(%s,O_RDONLY)", alloca_str_toprint(path));
int ret = fread(buffer, 1, buffer_size, f); ssize_t ret = read(fd, buffer, buffer_size);
if (ret == -1) if (ret == -1)
ret = WHY_perror("fread"); ret = WHYF_perror("read(%s,%u)", alloca_str_toprint(path), buffer_size);
if (fclose(f) == EOF) if (close(fd) == -1)
ret = WHY_perror("fclose"); ret = WHY_perror("close");
return ret; return ret;
} }
@ -325,7 +325,7 @@ int rhizome_manifest_parse(rhizome_manifest *m)
OUT(); OUT();
} }
int rhizome_read_manifest_file(rhizome_manifest *m, const char *filename, int bufferP) int rhizome_read_manifest_file(rhizome_manifest *m, const char *filename, size_t bufferP)
{ {
if (!m) if (!m)
return WHY("Null manifest"); return WHY("Null manifest");
@ -336,9 +336,10 @@ int rhizome_read_manifest_file(rhizome_manifest *m, const char *filename, int bu
m->manifest_bytes=bufferP; m->manifest_bytes=bufferP;
memcpy(m->manifestdata, filename, m->manifest_bytes); memcpy(m->manifestdata, filename, m->manifest_bytes);
} else { } else {
m->manifest_bytes = read_whole_file(filename, m->manifestdata, sizeof(m->manifestdata)); ssize_t bytes = read_whole_file(filename, m->manifestdata, sizeof m->manifestdata);
if (m->manifest_bytes == -1) if (bytes == -1)
return -1; return -1;
m->manifest_bytes = bytes;
} }
return rhizome_manifest_parse(m); return rhizome_manifest_parse(m);
} }
@ -643,30 +644,30 @@ int rhizome_manifest_selfsign(rhizome_manifest *m)
return 0; return 0;
} }
int rhizome_write_manifest_file(rhizome_manifest *m, const char *filename, char append) int rhizome_write_manifest_file(rhizome_manifest *m, const char *path, char append)
{ {
if (config.debug.rhizome) DEBUGF("write manifest (%d bytes) to %s", m->manifest_all_bytes, filename); if (config.debug.rhizome)
if (!m) return WHY("Manifest is null."); DEBUGF("write manifest (%d bytes) to %s", m->manifest_all_bytes, path);
if (!m->finalised) return WHY("Manifest must be finalised before it can be written."); if (!m)
FILE *f = fopen(filename, (append?"a":"w")); return WHY("Manifest is null.");
if (f == NULL) if (!m->finalised)
return WHYF_perror("Cannot write manifest to %s", filename); return WHY("Manifest must be finalised before it can be written.");
int fd = open(path, O_WRONLY | O_CREAT | (append ? O_APPEND : 0), 0666);
if (fd == -1)
return WHYF_perror("open(%s,O_WRONLY|O_CREAT%s,0666)", alloca_str_toprint(path), append ? "|O_APPEND" : "");
int ret = 0; int ret = 0;
if (fwrite(m->manifestdata, m->manifest_all_bytes, 1, f)!=1) if (write_all(fd, m->manifestdata, m->manifest_all_bytes) == -1)
ret=WHYF_perror("fwrite(%s)", filename); ret = -1;
else if (append) {
if (ret==0 && append){
unsigned char marker[4]; unsigned char marker[4];
write_uint16(marker, m->manifest_all_bytes); write_uint16(marker, m->manifest_all_bytes);
marker[2]=0x41; marker[2]=0x41;
marker[3]=0x10; marker[3]=0x10;
if (fwrite(marker, 4,1,f)!=1) if (write_all(fd, marker, sizeof marker) == -1)
ret=WHYF_perror("fwrite(%s)", filename); ret = -1;
} }
if (close(fd) == -1)
if (fclose(f)) ret = WHY_perror("close");
ret=WHYF_perror("fclose(%s)", filename);
return ret; return ret;
} }

View File

@ -151,7 +151,7 @@ void verify_bundles(){
while(sqlite_step_retry(&retry, statement)==SQLITE_ROW){ while(sqlite_step_retry(&retry, statement)==SQLITE_ROW){
sqlite3_int64 rowid = sqlite3_column_int64(statement, 0); sqlite3_int64 rowid = sqlite3_column_int64(statement, 0);
const void *manifest = sqlite3_column_blob(statement, 1); const void *manifest = sqlite3_column_blob(statement, 1);
int manifest_length = sqlite3_column_bytes(statement, 1); size_t manifest_length = sqlite3_column_bytes(statement, 1);
rhizome_manifest *m=rhizome_new_manifest(); rhizome_manifest *m=rhizome_new_manifest();
int ret=0; int ret=0;
@ -1734,7 +1734,7 @@ static int unpack_manifest_row(rhizome_manifest *m, sqlite3_stmt *statement)
int64_t q_inserttime = sqlite3_column_int64(statement, 3); int64_t q_inserttime = sqlite3_column_int64(statement, 3);
const char *q_author = (const char *) sqlite3_column_text(statement, 4); const char *q_author = (const char *) sqlite3_column_text(statement, 4);
size_t q_blobsize = sqlite3_column_bytes(statement, 1); // must call after sqlite3_column_blob() size_t q_blobsize = sqlite3_column_bytes(statement, 1); // must call after sqlite3_column_blob()
if (rhizome_read_manifest_file(m, q_blob, q_blobsize)) if (rhizome_read_manifest_file(m, q_blob, q_blobsize) == -1)
return WHYF("Manifest %s exists but is invalid", q_id); return WHYF("Manifest %s exists but is invalid", q_id);
if (q_author) { if (q_author) {
if (str_to_sid_t(&m->author, q_author) == -1) if (str_to_sid_t(&m->author, q_author) == -1)

View File

@ -208,11 +208,14 @@ int rhizome_direct_form_received(rhizome_http_request *r)
snprintf(filepath,1024,"rhizomedirect.%d.data",r->alarm.poll.fd); snprintf(filepath,1024,"rhizomedirect.%d.data",r->alarm.poll.fd);
char manifestTemplate[1024]; char manifestTemplate[1024];
manifestTemplate[0] = '\0';
if (config.rhizome.api.addfile.manifest_template_file[0]) {
strbuf b = strbuf_local(manifestTemplate, sizeof manifestTemplate); strbuf b = strbuf_local(manifestTemplate, sizeof manifestTemplate);
strbuf_path_join(b, serval_instancepath(), config.rhizome.api.addfile.manifest_template_file, NULL); strbuf_path_join(b, serval_instancepath(), config.rhizome.api.addfile.manifest_template_file, NULL);
if (manifestTemplate[0] && access(manifestTemplate, R_OK) != 0) { if (access(manifestTemplate, R_OK) != 0) {
rhizome_direct_clear_temporary_files(r); rhizome_direct_clear_temporary_files(r);
return rhizome_server_simple_http_response(r,500,"rhizome.api.addfile.manifesttemplate points to a file I could not read."); return rhizome_server_simple_http_response(r,500,"rhizome.api.addfile.manifest_template_file points to a file I could not read.");
}
} }
rhizome_manifest *m = rhizome_new_manifest(); rhizome_manifest *m = rhizome_new_manifest();

View File

@ -1287,8 +1287,7 @@ int rhizome_write_complete(struct rhizome_fetch_slot *slot)
call schedule queued items. */ call schedule queued items. */
rhizome_manifest *m = rhizome_new_manifest(); rhizome_manifest *m = rhizome_new_manifest();
if (m) { if (m) {
if (rhizome_read_manifest_file(m, slot->manifest_buffer, if (rhizome_read_manifest_file(m, slot->manifest_buffer, (size_t)slot->manifest_bytes) == -1) {
slot->manifest_bytes) == -1) {
DEBUGF("Couldn't read manifest"); DEBUGF("Couldn't read manifest");
rhizome_manifest_free(m); rhizome_manifest_free(m);
} else { } else {

View File

@ -290,7 +290,7 @@ int overlay_rhizome_saw_advertisements(int i, struct decode_context *context, st
int ad_frame_type=ob_get(f->payload); int ad_frame_type=ob_get(f->payload);
struct sockaddr_in httpaddr = context->addr; struct sockaddr_in httpaddr = context->addr;
httpaddr.sin_port = htons(RHIZOME_HTTP_PORT); httpaddr.sin_port = htons(RHIZOME_HTTP_PORT);
int manifest_length; size_t manifest_length;
rhizome_manifest *m=NULL; rhizome_manifest *m=NULL;
int (*oldfunc)() = sqlite_set_tracefunc(is_debug_rhizome_ads); int (*oldfunc)() = sqlite_set_tracefunc(is_debug_rhizome_ads);
@ -312,7 +312,7 @@ int overlay_rhizome_saw_advertisements(int i, struct decode_context *context, st
unsigned char *data = ob_get_bytes_ptr(f->payload, manifest_length); unsigned char *data = ob_get_bytes_ptr(f->payload, manifest_length);
if (!data) { if (!data) {
WHYF("Illegal manifest length field in rhizome advertisement frame %d vs %d.", WHYF("Illegal manifest length field in rhizome advertisement frame %zu vs %d",
manifest_length, f->payload->sizeLimit - f->payload->position); manifest_length, f->payload->sizeLimit - f->payload->position);
break; break;
} }