[pacman-dev] [PATCH 3/3] Unify buffer size used across the library

[Dave Reisner]

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 at 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
> 
>