[pacman-dev] [PATCH 1/3] libalpm/util: two stat() related cleanups
First, use fstat() in preference to stat() since we already have an open file handle. This also removes the need to check for a symlink as that is not possible when a file is opened. Next, use archive_entry_mode() rather than archive_entry_stat() as we only use the mode portion of the stat struct and the call is much cheaper. Also delay it until it is necessary. Signed-off-by: Dan McGee <dan@archlinux.org> --- lib/libalpm/util.c | 27 +++++++++++++-------------- 1 files changed, 13 insertions(+), 14 deletions(-) diff --git a/lib/libalpm/util.c b/lib/libalpm/util.c index 0d499ad..af9cf42 100644 --- a/lib/libalpm/util.c +++ b/lib/libalpm/util.c @@ -160,12 +160,10 @@ int _alpm_copyfile(const char *src, const char *dest) } } - /* chmod dest to permissions of src, as long as it is not a symlink */ + /* chmod dest to permissions of src */ struct stat statbuf; - if(!stat(src, &statbuf)) { - if(! S_ISLNK(statbuf.st_mode)) { - fchmod(fileno(out), statbuf.st_mode); - } + if(!fstat(fileno(in), &statbuf)) { + fchmod(fileno(out), statbuf.st_mode); } else { /* stat was unsuccessful */ ret = 1; @@ -313,18 +311,11 @@ int _alpm_unpack(alpm_handle_t *handle, const char *archive, const char *prefix, } while(archive_read_next_header(_archive, &entry) == ARCHIVE_OK) { - const struct stat *st; - const char *entryname; /* the name of the file in the archive */ + const char *entryname; + mode_t mode; - st = archive_entry_stat(entry); entryname = archive_entry_pathname(entry); - if(S_ISREG(st->st_mode)) { - archive_entry_set_perm(entry, 0644); - } else if(S_ISDIR(st->st_mode)) { - archive_entry_set_perm(entry, 0755); - } - /* If specific files were requested, skip entries that don't match. */ if(list) { char *entry_prefix = strdup(entryname); @@ -345,6 +336,13 @@ int _alpm_unpack(alpm_handle_t *handle, const char *archive, const char *prefix, } } + mode = archive_entry_mode(entry); + if(S_ISREG(mode)) { + archive_entry_set_perm(entry, 0644); + } else if(S_ISDIR(mode)) { + archive_entry_set_perm(entry, 0755); + } + /* Extract the archive entry. */ int readret = archive_read_extract(_archive, entry, 0); if(readret == ARCHIVE_WARN) { @@ -555,6 +553,7 @@ int _alpm_run_chroot(alpm_handle_t *handle, const char *path, char *const argv[] } umask(0022); execv(path, argv); + /* execv only returns if there was an error */ fprintf(stderr, _("call to execv failed (%s)\n"), strerror(errno)); exit(1); } else { -- 1.7.7
This gives us a bit more control and over the archive reading process, and a bit less is done behind the scenes. It also allows us to use fstat() in preference to stat(), which should avoid some potential race conditions. Some reorganization is necessary to move the stat calls after the open() calls. Error handling and cleanup in general is also improved, as we had several potential memory and file handle leaks before in some error paths. Signed-off-by: Dan McGee <dan@archlinux.org> --- lib/libalpm/be_package.c | 53 +++++++++++++++++++++++++++++--------------- lib/libalpm/be_sync.c | 55 +++++++++++++++++++++++++++++---------------- 2 files changed, 70 insertions(+), 38 deletions(-) diff --git a/lib/libalpm/be_package.c b/lib/libalpm/be_package.c index c20c703..36666df 100644 --- a/lib/libalpm/be_package.c +++ b/lib/libalpm/be_package.c @@ -23,6 +23,9 @@ #include <stdlib.h> #include <string.h> #include <errno.h> +#include <sys/types.h> +#include <sys/stat.h> +#include <fcntl.h> /* libarchive */ #include <archive.h> @@ -355,7 +358,7 @@ int _alpm_pkg_validate_internal(alpm_handle_t *handle, alpm_pkg_t *_alpm_pkg_load_internal(alpm_handle_t *handle, const char *pkgfile, int full) { - int ret, config = 0; + int ret, fd, config = 0; struct archive *archive; struct archive_entry *entry; alpm_pkg_t *newpkg = NULL; @@ -367,34 +370,44 @@ alpm_pkg_t *_alpm_pkg_load_internal(alpm_handle_t *handle, RET_ERR(handle, ALPM_ERR_WRONG_ARGS, NULL); } - /* attempt to stat the package file, ensure it exists */ - if(stat(pkgfile, &st) == 0) { - newpkg = _alpm_pkg_new(); - if(newpkg == NULL) { - RET_ERR(handle, ALPM_ERR_MEMORY, NULL); - } - newpkg->filename = strdup(pkgfile); - newpkg->size = st.st_size; - } else { - /* couldn't stat the pkgfile, return an error */ - RET_ERR(handle, ALPM_ERR_PKG_NOT_FOUND, NULL); - } - /* try to create an archive object to read in the package */ if((archive = archive_read_new()) == NULL) { - alpm_pkg_free(newpkg); RET_ERR(handle, ALPM_ERR_LIBARCHIVE, NULL); } archive_read_support_compression_all(archive); archive_read_support_format_all(archive); - if(archive_read_open_filename(archive, pkgfile, + do { + fd = open(pkgfile, O_RDONLY); + } while(fd == -1 && errno == EINTR); + + if(fd < 0 || archive_read_open_fd(archive, fd, ARCHIVE_DEFAULT_BYTES_PER_BLOCK) != ARCHIVE_OK) { - alpm_pkg_free(newpkg); - RET_ERR(handle, ALPM_ERR_PKG_OPEN, NULL); + const char *err = fd < 0 ? strerror(errno) : archive_error_string(archive); + _alpm_log(handle, ALPM_LOG_ERROR, + _("could not open file %s: %s\n"), pkgfile, err); + if(fd < 0 && errno == ENOENT) { + handle->pm_errno = ALPM_ERR_PKG_NOT_FOUND; + } else { + handle->pm_errno = ALPM_ERR_PKG_OPEN; + } + goto error; } + newpkg = _alpm_pkg_new(); + if(newpkg == NULL) { + handle->pm_errno = ALPM_ERR_MEMORY; + goto error; + } + if(fstat(fd, &st) != 0) { + handle->pm_errno = ALPM_ERR_PKG_OPEN; + goto error; + } + STRDUP(newpkg->filename, pkgfile, + handle->pm_errno = ALPM_ERR_MEMORY; goto error); + newpkg->size = st.st_size; + _alpm_log(handle, ALPM_LOG_DEBUG, "starting package load for %s\n", pkgfile); /* If full is false, only read through the archive until we find our needed @@ -476,6 +489,7 @@ alpm_pkg_t *_alpm_pkg_load_internal(alpm_handle_t *handle, } archive_read_finish(archive); + close(fd); /* internal fields for package struct */ newpkg->origin = PKG_FROM_FILE; @@ -502,6 +516,9 @@ pkg_invalid: error: _alpm_pkg_free(newpkg); archive_read_finish(archive); + if(fd >= 0) { + close(fd); + } return NULL; } diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c index e9e816c..24785f1 100644 --- a/lib/libalpm/be_sync.c +++ b/lib/libalpm/be_sync.c @@ -21,7 +21,9 @@ #include "config.h" #include <errno.h> +#include <sys/types.h> #include <sys/stat.h> +#include <fcntl.h> #include <unistd.h> /* libarchive */ @@ -212,6 +214,7 @@ int SYMEXPORT alpm_db_update(int force, alpm_db_t *db) /* print server + filename into a buffer */ len = strlen(server) + strlen(db->treename) + 5; + /* TODO fix leak syncpath and umask unset */ MALLOC(payload.fileurl, len, RET_ERR(handle, ALPM_ERR_MEMORY, -1)); snprintf(payload.fileurl, len, "%s/%s.db", server, db->treename); payload.handle = handle; @@ -234,6 +237,7 @@ int SYMEXPORT alpm_db_update(int force, alpm_db_t *db) /* if we downloaded a DB, we want the .sig from the same server */ /* print server + filename into a buffer (leave space for .sig) */ len = strlen(server) + strlen(db->treename) + 9; + /* TODO fix leak syncpath and umask unset */ MALLOC(payload.fileurl, len, RET_ERR(handle, ALPM_ERR_MEMORY, -1)); snprintf(payload.fileurl, len, "%s/%s.db.sig", server, db->treename); payload.handle = handle; @@ -411,7 +415,7 @@ static int sync_db_populate(alpm_db_t *db) { const char *dbpath; size_t est_count; - int count = 0; + int count = -1, fd; struct stat buf; struct archive *archive; struct archive_entry *entry; @@ -423,6 +427,11 @@ static int sync_db_populate(alpm_db_t *db) if(db->status & DB_STATUS_MISSING) { RET_ERR(db->handle, ALPM_ERR_DB_NOT_FOUND, -1); } + dbpath = _alpm_db_path(db); + if(!dbpath) { + /* pm_errno set in _alpm_db_path() */ + return -1; + } if((archive = archive_read_new()) == NULL) { RET_ERR(db->handle, ALPM_ERR_LIBARCHIVE, -1); @@ -431,30 +440,31 @@ static int sync_db_populate(alpm_db_t *db) archive_read_support_compression_all(archive); archive_read_support_format_all(archive); - dbpath = _alpm_db_path(db); - if(!dbpath) { - /* pm_errno set in _alpm_db_path() */ - return -1; - } - - _alpm_log(db->handle, ALPM_LOG_DEBUG, "opening database archive %s\n", dbpath); + _alpm_log(db->handle, ALPM_LOG_DEBUG, + "opening database archive %s\n", dbpath); + do { + fd = open(dbpath, O_RDONLY); + } while(fd == -1 && errno == EINTR); - if(archive_read_open_filename(archive, dbpath, + if(fd < 0 || archive_read_open_fd(archive, fd, ARCHIVE_DEFAULT_BYTES_PER_BLOCK) != ARCHIVE_OK) { - _alpm_log(db->handle, ALPM_LOG_ERROR, _("could not open file %s: %s\n"), dbpath, - archive_error_string(archive)); - archive_read_finish(archive); - RET_ERR(db->handle, ALPM_ERR_DB_OPEN, -1); + const char *err = fd < 0 ? strerror(errno) : archive_error_string(archive); + _alpm_log(db->handle, ALPM_LOG_ERROR, + _("could not open file %s: %s\n"), dbpath, err); + db->handle->pm_errno = ALPM_ERR_DB_OPEN; + goto cleanup; } - if(stat(dbpath, &buf) != 0) { - RET_ERR(db->handle, ALPM_ERR_DB_OPEN, -1); + if(fstat(fd, &buf) != 0) { + db->handle->pm_errno = ALPM_ERR_DB_OPEN; + goto cleanup; } est_count = estimate_package_count(&buf, archive); /* initialize hash at 66% full */ db->pkgcache = _alpm_pkghash_create(est_count * 3 / 2); if(db->pkgcache == NULL) { - RET_ERR(db->handle, ALPM_ERR_MEMORY, -1); + db->handle->pm_errno = ALPM_ERR_MEMORY; + goto cleanup; } while(archive_read_next_header(archive, &entry) == ARCHIVE_OK) { @@ -473,14 +483,19 @@ static int sync_db_populate(alpm_db_t *db) } count = alpm_list_count(db->pkgcache->list); - if(count > 0) { - db->pkgcache->list = alpm_list_msort(db->pkgcache->list, (size_t)count, _alpm_pkg_cmp); + db->pkgcache->list = alpm_list_msort(db->pkgcache->list, + (size_t)count, _alpm_pkg_cmp); } - archive_read_finish(archive); - _alpm_log(db->handle, ALPM_LOG_DEBUG, "added %d packages to package cache for db '%s'\n", + _alpm_log(db->handle, ALPM_LOG_DEBUG, + "added %d packages to package cache for db '%s'\n", count, db->treename); +cleanup: + archive_read_finish(archive); + if(fd >= 0) { + close(fd); + } return count; } -- 1.7.7
Define a single BUFFER_SIZE constant in util.h and use it everywhere. This also prepares us for libarchive 3.0, where the ARCHIVE_DEFAULT_BYTES_PER_BLOCK constant has been taken out to the woodshed. 16384 was pulled out of the air; I figured we can spare 16K of memory at a time, vs. the usual used 8K before. Signed-off-by: Dan McGee <dan@archlinux.org> --- I wrote this on master, but it might be wise to port it to maint and be proactive so old releases don't suddenly break and need patching with a new libarchive. Pretty low-risk here. lib/libalpm/add.c | 2 +- lib/libalpm/be_package.c | 6 ++---- lib/libalpm/be_sync.c | 3 +-- lib/libalpm/util.c | 13 ++++--------- lib/libalpm/util.h | 3 +++ 5 files changed, 11 insertions(+), 16 deletions(-) diff --git a/lib/libalpm/add.c b/lib/libalpm/add.c index 2ef4178..e6ac4e1 100644 --- a/lib/libalpm/add.c +++ b/lib/libalpm/add.c @@ -537,7 +537,7 @@ static int commit_single_pkg(alpm_handle_t *handle, alpm_pkg_t *newpkg, _alpm_log(handle, ALPM_LOG_DEBUG, "archive: %s\n", newpkg->origin_data.file); if(archive_read_open_filename(archive, newpkg->origin_data.file, - ARCHIVE_DEFAULT_BYTES_PER_BLOCK) != ARCHIVE_OK) { + BUFFER_SIZE) != ARCHIVE_OK) { handle->pm_errno = ALPM_ERR_PKG_OPEN; ret = -1; goto cleanup; diff --git a/lib/libalpm/be_package.c b/lib/libalpm/be_package.c index 36666df..3d76afe 100644 --- a/lib/libalpm/be_package.c +++ b/lib/libalpm/be_package.c @@ -61,8 +61,7 @@ static void *_package_changelog_open(alpm_pkg_t *pkg) archive_read_support_compression_all(archive); archive_read_support_format_all(archive); - if(archive_read_open_filename(archive, pkgfile, - ARCHIVE_DEFAULT_BYTES_PER_BLOCK) != ARCHIVE_OK) { + if(archive_read_open_filename(archive, pkgfile, BUFFER_SIZE) != ARCHIVE_OK) { RET_ERR(pkg->handle, ALPM_ERR_PKG_OPEN, NULL); } @@ -382,8 +381,7 @@ alpm_pkg_t *_alpm_pkg_load_internal(alpm_handle_t *handle, fd = open(pkgfile, O_RDONLY); } while(fd == -1 && errno == EINTR); - if(fd < 0 || archive_read_open_fd(archive, fd, - ARCHIVE_DEFAULT_BYTES_PER_BLOCK) != ARCHIVE_OK) { + if(fd < 0 || archive_read_open_fd(archive, fd, BUFFER_SIZE) != ARCHIVE_OK) { const char *err = fd < 0 ? strerror(errno) : archive_error_string(archive); _alpm_log(handle, ALPM_LOG_ERROR, _("could not open file %s: %s\n"), pkgfile, err); diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c index 24785f1..eff1ca0 100644 --- a/lib/libalpm/be_sync.c +++ b/lib/libalpm/be_sync.c @@ -446,8 +446,7 @@ static int sync_db_populate(alpm_db_t *db) fd = open(dbpath, O_RDONLY); } while(fd == -1 && errno == EINTR); - if(fd < 0 || archive_read_open_fd(archive, fd, - ARCHIVE_DEFAULT_BYTES_PER_BLOCK) != ARCHIVE_OK) { + if(fd < 0 || archive_read_open_fd(archive, fd, BUFFER_SIZE) != ARCHIVE_OK) { const char *err = fd < 0 ? strerror(errno) : archive_error_string(archive); _alpm_log(db->handle, ALPM_LOG_ERROR, _("could not open file %s: %s\n"), dbpath, err); diff --git a/lib/libalpm/util.c b/lib/libalpm/util.c index af9cf42..a5ed977 100644 --- a/lib/libalpm/util.c +++ b/lib/libalpm/util.c @@ -129,8 +129,6 @@ int _alpm_makepath_mode(const char *path, mode_t mode) return ret; } -#define CPBUFSIZE 8 * 1024 - int _alpm_copyfile(const char *src, const char *dest) { FILE *in, *out; @@ -148,10 +146,10 @@ int _alpm_copyfile(const char *src, const char *dest) return 1; } - CALLOC(buf, (size_t)CPBUFSIZE, (size_t)1, ret = 1; goto cleanup;); + MALLOC(buf, (size_t)BUFFER_SIZE, ret = 1; goto cleanup;); /* do the actual file copy */ - while((len = fread(buf, 1, CPBUFSIZE, in))) { + while((len = fread(buf, 1, BUFFER_SIZE, in))) { size_t nwritten = 0; nwritten = fwrite(buf, 1, len, out); if((nwritten != len) || ferror(out)) { @@ -172,7 +170,7 @@ int _alpm_copyfile(const char *src, const char *dest) cleanup: fclose(in); fclose(out); - FREE(buf); + free(buf); return ret; } @@ -285,8 +283,7 @@ int _alpm_unpack(alpm_handle_t *handle, const char *archive, const char *prefix, archive_read_support_compression_all(_archive); archive_read_support_format_all(_archive); - if(archive_read_open_filename(_archive, archive, - ARCHIVE_DEFAULT_BYTES_PER_BLOCK) != ARCHIVE_OK) { + if(archive_read_open_filename(_archive, archive, BUFFER_SIZE) != ARCHIVE_OK) { _alpm_log(handle, ALPM_LOG_ERROR, _("could not open file %s: %s\n"), archive, archive_error_string(_archive)); RET_ERR(handle, ALPM_ERR_PKG_OPEN, 1); @@ -740,8 +737,6 @@ int _alpm_lstat(const char *path, struct stat *buf) } #ifdef HAVE_LIBSSL -#define BUFFER_SIZE 8192 - static int md5_file(const char *path, unsigned char output[16]) { FILE *f; diff --git a/lib/libalpm/util.h b/lib/libalpm/util.h index 400a4ee..2514946 100644 --- a/lib/libalpm/util.h +++ b/lib/libalpm/util.h @@ -74,6 +74,9 @@ #define CHECK_HANDLE(handle, action) do { if(!(handle)) { action; } (handle)->pm_errno = 0; } while(0) +/** Standard buffer size used throughout the library. */ +#define BUFFER_SIZE 16384 + /** * Used as a buffer/state holder for _alpm_archive_fgets(). */ -- 1.7.7
On Wed, Oct 26, 2011 at 05:44:43PM -0500, Dan McGee wrote:
Define a single BUFFER_SIZE constant in util.h and use it everywhere. This also prepares us for libarchive 3.0, where the ARCHIVE_DEFAULT_BYTES_PER_BLOCK constant has been taken out to the woodshed.
Any particular reason we're not using BUFSIZ out of stdio.h if its available?
16384 was pulled out of the air; I figured we can spare 16K of memory at a time, vs. the usual used 8K before.
Signed-off-by: Dan McGee <dan@archlinux.org> ---
I wrote this on master, but it might be wise to port it to maint and be proactive so old releases don't suddenly break and need patching with a new libarchive. Pretty low-risk here.
lib/libalpm/add.c | 2 +- lib/libalpm/be_package.c | 6 ++---- lib/libalpm/be_sync.c | 3 +-- lib/libalpm/util.c | 13 ++++--------- lib/libalpm/util.h | 3 +++ 5 files changed, 11 insertions(+), 16 deletions(-)
diff --git a/lib/libalpm/add.c b/lib/libalpm/add.c index 2ef4178..e6ac4e1 100644 --- a/lib/libalpm/add.c +++ b/lib/libalpm/add.c @@ -537,7 +537,7 @@ static int commit_single_pkg(alpm_handle_t *handle, alpm_pkg_t *newpkg,
_alpm_log(handle, ALPM_LOG_DEBUG, "archive: %s\n", newpkg->origin_data.file); if(archive_read_open_filename(archive, newpkg->origin_data.file, - ARCHIVE_DEFAULT_BYTES_PER_BLOCK) != ARCHIVE_OK) { + BUFFER_SIZE) != ARCHIVE_OK) { handle->pm_errno = ALPM_ERR_PKG_OPEN; ret = -1; goto cleanup; diff --git a/lib/libalpm/be_package.c b/lib/libalpm/be_package.c index 36666df..3d76afe 100644 --- a/lib/libalpm/be_package.c +++ b/lib/libalpm/be_package.c @@ -61,8 +61,7 @@ static void *_package_changelog_open(alpm_pkg_t *pkg) archive_read_support_compression_all(archive); archive_read_support_format_all(archive);
- if(archive_read_open_filename(archive, pkgfile, - ARCHIVE_DEFAULT_BYTES_PER_BLOCK) != ARCHIVE_OK) { + if(archive_read_open_filename(archive, pkgfile, BUFFER_SIZE) != ARCHIVE_OK) { RET_ERR(pkg->handle, ALPM_ERR_PKG_OPEN, NULL); }
@@ -382,8 +381,7 @@ alpm_pkg_t *_alpm_pkg_load_internal(alpm_handle_t *handle, fd = open(pkgfile, O_RDONLY); } while(fd == -1 && errno == EINTR);
- if(fd < 0 || archive_read_open_fd(archive, fd, - ARCHIVE_DEFAULT_BYTES_PER_BLOCK) != ARCHIVE_OK) { + if(fd < 0 || archive_read_open_fd(archive, fd, BUFFER_SIZE) != ARCHIVE_OK) { const char *err = fd < 0 ? strerror(errno) : archive_error_string(archive); _alpm_log(handle, ALPM_LOG_ERROR, _("could not open file %s: %s\n"), pkgfile, err); diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c index 24785f1..eff1ca0 100644 --- a/lib/libalpm/be_sync.c +++ b/lib/libalpm/be_sync.c @@ -446,8 +446,7 @@ static int sync_db_populate(alpm_db_t *db) fd = open(dbpath, O_RDONLY); } while(fd == -1 && errno == EINTR);
- if(fd < 0 || archive_read_open_fd(archive, fd, - ARCHIVE_DEFAULT_BYTES_PER_BLOCK) != ARCHIVE_OK) { + if(fd < 0 || archive_read_open_fd(archive, fd, BUFFER_SIZE) != ARCHIVE_OK) { const char *err = fd < 0 ? strerror(errno) : archive_error_string(archive); _alpm_log(db->handle, ALPM_LOG_ERROR, _("could not open file %s: %s\n"), dbpath, err); diff --git a/lib/libalpm/util.c b/lib/libalpm/util.c index af9cf42..a5ed977 100644 --- a/lib/libalpm/util.c +++ b/lib/libalpm/util.c @@ -129,8 +129,6 @@ int _alpm_makepath_mode(const char *path, mode_t mode) return ret; }
-#define CPBUFSIZE 8 * 1024 - int _alpm_copyfile(const char *src, const char *dest) { FILE *in, *out; @@ -148,10 +146,10 @@ int _alpm_copyfile(const char *src, const char *dest) return 1; }
- CALLOC(buf, (size_t)CPBUFSIZE, (size_t)1, ret = 1; goto cleanup;); + MALLOC(buf, (size_t)BUFFER_SIZE, ret = 1; goto cleanup;);
/* do the actual file copy */ - while((len = fread(buf, 1, CPBUFSIZE, in))) { + while((len = fread(buf, 1, BUFFER_SIZE, in))) { size_t nwritten = 0; nwritten = fwrite(buf, 1, len, out); if((nwritten != len) || ferror(out)) { @@ -172,7 +170,7 @@ int _alpm_copyfile(const char *src, const char *dest) cleanup: fclose(in); fclose(out); - FREE(buf); + free(buf); return ret; }
@@ -285,8 +283,7 @@ int _alpm_unpack(alpm_handle_t *handle, const char *archive, const char *prefix, archive_read_support_compression_all(_archive); archive_read_support_format_all(_archive);
- if(archive_read_open_filename(_archive, archive, - ARCHIVE_DEFAULT_BYTES_PER_BLOCK) != ARCHIVE_OK) { + if(archive_read_open_filename(_archive, archive, BUFFER_SIZE) != ARCHIVE_OK) { _alpm_log(handle, ALPM_LOG_ERROR, _("could not open file %s: %s\n"), archive, archive_error_string(_archive)); RET_ERR(handle, ALPM_ERR_PKG_OPEN, 1); @@ -740,8 +737,6 @@ int _alpm_lstat(const char *path, struct stat *buf) }
#ifdef HAVE_LIBSSL -#define BUFFER_SIZE 8192 - static int md5_file(const char *path, unsigned char output[16]) { FILE *f; diff --git a/lib/libalpm/util.h b/lib/libalpm/util.h index 400a4ee..2514946 100644 --- a/lib/libalpm/util.h +++ b/lib/libalpm/util.h @@ -74,6 +74,9 @@
#define CHECK_HANDLE(handle, action) do { if(!(handle)) { action; } (handle)->pm_errno = 0; } while(0)
+/** Standard buffer size used throughout the library. */ +#define BUFFER_SIZE 16384 + /** * Used as a buffer/state holder for _alpm_archive_fgets(). */ -- 1.7.7
On Wed, Oct 26, 2011 at 6:16 PM, Dave Reisner <d@falconindy.com> wrote:
On Wed, Oct 26, 2011 at 05:44:43PM -0500, Dan McGee wrote:
Define a single BUFFER_SIZE constant in util.h and use it everywhere. This also prepares us for libarchive 3.0, where the ARCHIVE_DEFAULT_BYTES_PER_BLOCK constant has been taken out to the woodshed.
Any particular reason we're not using BUFSIZ out of stdio.h if its available?
1. It is documented only as "this is the size the functions in stdio.h use", not necessarily as "this is the buffer size user space applications should use". 2. Finding that documentation is hard. [1] 3. Any particular reason to? At least on my system it is 8192, and I guess I have no idea if that varies across systems. So yeah, I don't know. I suppose I could be convinced, and we could just ifdef a hardcoded value in there if BUFSIZ wasn't defined, but I also don't know that if that value has any sort of real magic to it. -Dan [1] The best I could find was <http://www.delorie.com/gnu/docs/glibc/libc_226.html>, and even that is glibc specific. Sometimes people also use BUFSIZ as the allocation size of buffers used for related purposes, such as strings used to receive a line of input with fgets (see section 12.8 Character Input). There is no particular reason to use BUFSIZ for this instead of any other integer, except that it might lead to doing I/O in chunks of an efficient size.
participants (3)
-
Dan McGee
-
Dan McGee
-
Dave Reisner