[pacman-dev] [PATCH 0/5] Random fixes from my working branch
Posting this random collection of patches before they grow hair and develop a personality more likeable than mine. Dave Reisner (5): add real_line_size to alpm_read_buffer add line length parameter to _alpm_strip_newline contrib: sed out @SCRIPTNAME@ in edit command updpkgsums: avoid fancy quoting in error message diskspace: log errors when opening the mount table fails contrib/Makefile.am | 1 + contrib/updpkgsums.sh.in | 2 +- lib/libalpm/be_local.c | 14 +++++++------- lib/libalpm/be_package.c | 2 +- lib/libalpm/be_sync.c | 8 ++++---- lib/libalpm/diskspace.c | 6 ++++++ lib/libalpm/util.c | 12 +++++++----- lib/libalpm/util.h | 3 ++- 8 files changed, 29 insertions(+), 19 deletions(-) -- 1.7.10.4
We inevitably call strlen() or similar on the line returned from _alpm_archive_fgets(), so include the line size of the interesting line in the struct. Signed-off-by: Dave Reisner <dreisner@archlinux.org> --- lib/libalpm/util.c | 4 ++-- lib/libalpm/util.h | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/libalpm/util.c b/lib/libalpm/util.c index 996d7f4..8fd7a10 100644 --- a/lib/libalpm/util.c +++ b/lib/libalpm/util.c @@ -1009,7 +1009,7 @@ int _alpm_archive_fgets(struct archive *a, struct archive_read_buffer *b) } if(eol) { - size_t len = (size_t)(eol - b->block_offset); + size_t len = b->real_line_size = (size_t)(eol - b->block_offset); memcpy(b->line_offset, b->block_offset, len); b->line_offset[len] = '\0'; b->block_offset = eol + 1; @@ -1017,7 +1017,7 @@ int _alpm_archive_fgets(struct archive *a, struct archive_read_buffer *b) return ARCHIVE_OK; } else { /* we've looked through the whole block but no newline, copy it */ - size_t len = (size_t)(b->block + b->block_size - b->block_offset); + size_t len = b->real_line_size = (size_t)(b->block + b->block_size - b->block_offset); memcpy(b->line_offset, b->block_offset, len); b->line_offset += len; b->block_offset = b->block + b->block_size; diff --git a/lib/libalpm/util.h b/lib/libalpm/util.h index 87d51ea..9bcd59e 100644 --- a/lib/libalpm/util.h +++ b/lib/libalpm/util.h @@ -97,6 +97,7 @@ struct archive_read_buffer { char *line_offset; size_t line_size; size_t max_line_size; + size_t real_line_size; char *block; char *block_offset; @@ -108,7 +109,7 @@ struct archive_read_buffer { int _alpm_makepath(const char *path); int _alpm_makepath_mode(const char *path, mode_t mode); int _alpm_copyfile(const char *src, const char *dest); -size_t _alpm_strip_newline(char *str); +size_t _alpm_strip_newline(char *str, size_t len); int _alpm_open_archive(alpm_handle_t *handle, const char *path, struct stat *buf, struct archive **archive, alpm_errno_t error); -- 1.7.10.4
If known, callers can pass the line size to this function in order to avoid an strlen call. Otherwise, they simply pass 0 and _alpm_strip_newline will do the call instead. Signed-off-by: Dave Reisner <dreisner@archlinux.org> --- lib/libalpm/be_local.c | 14 +++++++------- lib/libalpm/be_package.c | 2 +- lib/libalpm/be_sync.c | 8 ++++---- lib/libalpm/util.c | 8 +++++--- 4 files changed, 17 insertions(+), 15 deletions(-) diff --git a/lib/libalpm/be_local.c b/lib/libalpm/be_local.c index 30e59d0..ee805b9 100644 --- a/lib/libalpm/be_local.c +++ b/lib/libalpm/be_local.c @@ -496,7 +496,7 @@ char *_alpm_local_db_pkgpath(alpm_db_t *db, alpm_pkg_t *info, #define READ_NEXT() do { \ if(fgets(line, sizeof(line), fp) == NULL && !feof(fp)) goto error; \ - _alpm_strip_newline(line); \ + _alpm_strip_newline(line, 0); \ } while(0) #define READ_AND_STORE(f) do { \ @@ -509,7 +509,7 @@ char *_alpm_local_db_pkgpath(alpm_db_t *db, alpm_pkg_t *info, if(fgets(line, sizeof(line), fp) == NULL) {\ if(!feof(fp)) goto error; else break; \ } \ - if(_alpm_strip_newline(line) == 0) break; \ + if(_alpm_strip_newline(line, 0) == 0) break; \ STRDUP(linedup, line, goto error); \ f = alpm_list_add(f, linedup); \ } while(1) /* note the while(1) and not (0) */ @@ -518,7 +518,7 @@ char *_alpm_local_db_pkgpath(alpm_db_t *db, alpm_pkg_t *info, if(fgets(line, sizeof(line), fp) == NULL) {\ if(!feof(fp)) goto error; else break; \ } \ - if(_alpm_strip_newline(line) == 0) break; \ + if(_alpm_strip_newline(line, 0) == 0) break; \ f = alpm_list_add(f, _alpm_splitdep(line)); \ } while(1) /* note the while(1) and not (0) */ @@ -564,7 +564,7 @@ static int local_db_read(alpm_pkg_t *info, alpm_dbinfrq_t inforeq) if(fgets(line, sizeof(line), fp) == NULL && !feof(fp)) { goto error; } - if(_alpm_strip_newline(line) == 0) { + if(_alpm_strip_newline(line, 0) == 0) { /* length of stripped line was zero */ continue; } @@ -651,13 +651,13 @@ static int local_db_read(alpm_pkg_t *info, alpm_dbinfrq_t inforeq) } free(path); while(fgets(line, sizeof(line), fp)) { - _alpm_strip_newline(line); + _alpm_strip_newline(line, 0); if(strcmp(line, "%FILES%") == 0) { size_t files_count = 0, files_size = 0, len; alpm_file_t *files = NULL; while(fgets(line, sizeof(line), fp) && - (len = _alpm_strip_newline(line))) { + (len = _alpm_strip_newline(line, 0))) { if(files_count >= files_size) { size_t old_size = files_size; if(files_size == 0) { @@ -691,7 +691,7 @@ static int local_db_read(alpm_pkg_t *info, alpm_dbinfrq_t inforeq) info->files.count = files_count; info->files.files = files; } else if(strcmp(line, "%BACKUP%") == 0) { - while(fgets(line, sizeof(line), fp) && _alpm_strip_newline(line)) { + while(fgets(line, sizeof(line), fp) && _alpm_strip_newline(line, 0)) { alpm_backup_t *backup; CALLOC(backup, 1, sizeof(alpm_backup_t), goto error); if(_alpm_split_backup(line, &backup)) { diff --git a/lib/libalpm/be_package.c b/lib/libalpm/be_package.c index 0d296a8..dd027f5 100644 --- a/lib/libalpm/be_package.c +++ b/lib/libalpm/be_package.c @@ -168,7 +168,7 @@ static int parse_descfile(alpm_handle_t *handle, struct archive *a, alpm_pkg_t * /* loop until we reach EOF or other error */ while((ret = _alpm_archive_fgets(a, &buf)) == ARCHIVE_OK) { - size_t len = _alpm_strip_newline(buf.line); + size_t len = _alpm_strip_newline(buf.line, buf.real_line_size); linenum++; key = buf.line; diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c index 02f087a..29ef190 100644 --- a/lib/libalpm/be_sync.c +++ b/lib/libalpm/be_sync.c @@ -482,7 +482,7 @@ cleanup: #define READ_NEXT() do { \ if(_alpm_archive_fgets(archive, &buf) != ARCHIVE_OK) goto error; \ line = buf.line; \ - _alpm_strip_newline(line); \ + _alpm_strip_newline(line, buf.real_line_size); \ } while(0) #define READ_AND_STORE(f) do { \ @@ -493,14 +493,14 @@ cleanup: #define READ_AND_STORE_ALL(f) do { \ char *linedup; \ if(_alpm_archive_fgets(archive, &buf) != ARCHIVE_OK) goto error; \ - if(_alpm_strip_newline(buf.line) == 0) break; \ + if(_alpm_strip_newline(buf.line, buf.real_line_size) == 0) break; \ STRDUP(linedup, buf.line, goto error); \ f = alpm_list_add(f, linedup); \ } while(1) /* note the while(1) and not (0) */ #define READ_AND_SPLITDEP(f) do { \ if(_alpm_archive_fgets(archive, &buf) != ARCHIVE_OK) goto error; \ - if(_alpm_strip_newline(buf.line) == 0) break; \ + if(_alpm_strip_newline(buf.line, buf.real_line_size) == 0) break; \ f = alpm_list_add(f, _alpm_splitdep(line)); \ } while(1) /* note the while(1) and not (0) */ @@ -539,7 +539,7 @@ static int sync_db_read(alpm_db_t *db, struct archive *archive, int ret; while((ret = _alpm_archive_fgets(archive, &buf)) == ARCHIVE_OK) { char *line = buf.line; - if(_alpm_strip_newline(line) == 0) { + if(_alpm_strip_newline(line, buf.real_line_size) == 0) { /* length of stripped line was zero */ continue; } diff --git a/lib/libalpm/util.c b/lib/libalpm/util.c index 8fd7a10..4877ff5 100644 --- a/lib/libalpm/util.c +++ b/lib/libalpm/util.c @@ -195,15 +195,17 @@ cleanup: /** Trim trailing newlines from a string (if any exist). * @param str a single line of text + * @param len size of str, if known, else 0 * @return the length of the trimmed string */ -size_t _alpm_strip_newline(char *str) +size_t _alpm_strip_newline(char *str, size_t len) { - size_t len; if(*str == '\0') { return 0; } - len = strlen(str); + if(len == 0) { + len = strlen(str); + } while(len > 0 && str[len - 1] == '\n') { len--; } -- 1.7.10.4
Signed-off-by: Dave Reisner <dreisner@archlinux.org> --- contrib/Makefile.am | 1 + 1 file changed, 1 insertion(+) diff --git a/contrib/Makefile.am b/contrib/Makefile.am index 30a76df..3641a08 100644 --- a/contrib/Makefile.am +++ b/contrib/Makefile.am @@ -58,6 +58,7 @@ edit = sed \ -e 's|@localstatedir[@]|$(localstatedir)|g' \ -e 's|@PACKAGE_VERSION[@]|$(REAL_PACKAGE_VERSION)|g' \ -e 's|@SIZECMD[@]|$(SIZECMD)|g' \ + -e 's|@SCRIPTNAME[@]|$@|g' \ -e '1s|!/bin/bash|!$(BASH_SHELL)|g' $(OTHERSCRIPTS): Makefile -- 1.7.10.4
m4 has a field day parsing escapes and actually vandalizes this string, causing the error to look like: ==> ERROR: \PKGBUILD\ not found or is not a file Avoid all quoting and just match up with how makepkg reports errors (no quoting at all). Signed-off-by: Dave Reisner <dreisner@archlinux.org> --- contrib/updpkgsums.sh.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/updpkgsums.sh.in b/contrib/updpkgsums.sh.in index f4649b5..38c8281 100755 --- a/contrib/updpkgsums.sh.in +++ b/contrib/updpkgsums.sh.in @@ -43,7 +43,7 @@ esac buildfile=${1:-PKGBUILD} if [[ ! -f $buildfile ]]; then - printf $'==> ERROR: \`%s\' not found or is not a file: %s\n' "$buildfile" + printf '==> ERROR: %s not found or is not a file: %s\n' "$buildfile" exit 1 fi -- 1.7.10.4
Signed-off-by: Dave Reisner <dreisner@archlinux.org> --- lib/libalpm/diskspace.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/libalpm/diskspace.c b/lib/libalpm/diskspace.c index 1fc297e..df2bb71 100644 --- a/lib/libalpm/diskspace.c +++ b/lib/libalpm/diskspace.c @@ -82,6 +82,8 @@ static alpm_list_t *mount_point_list(alpm_handle_t *handle) fp = setmntent(MOUNTED, "r"); if(fp == NULL) { + _alpm_log(handle, ALPM_LOG_ERROR, "failed to open file: %s: %s\n", + MOUNTED, strerror(errno)); return NULL; } @@ -118,6 +120,8 @@ static alpm_list_t *mount_point_list(alpm_handle_t *handle) fp = fopen("/etc/mnttab", "r"); if(fp == NULL) { + _alpm_log(handle, ALPM_LOG_ERROR, "failed to open file: /etc/mnttab: %s\n", + strerror(errno)); return NULL; } @@ -153,6 +157,8 @@ static alpm_list_t *mount_point_list(alpm_handle_t *handle) entries = getmntinfo(&fsp, MNT_NOWAIT); if(entries < 0) { + _alpm_log(handle, ALPM_LOG_ERROR, + "failed to open mount table for reading: %s\n", strerror(errno)); return NULL; } -- 1.7.10.4
Signed-off-by: Dave Reisner <dreisner@archlinux.org> --- lib/libalpm/diskspace.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/lib/libalpm/diskspace.c b/lib/libalpm/diskspace.c index 1fc297e..df2bb71 100644 --- a/lib/libalpm/diskspace.c +++ b/lib/libalpm/diskspace.c @@ -82,6 +82,8 @@ static alpm_list_t *mount_point_list(alpm_handle_t *handle) fp = setmntent(MOUNTED, "r");
if(fp == NULL) { + _alpm_log(handle, ALPM_LOG_ERROR, "failed to open file: %s: %s\n", + MOUNTED, strerror(errno)); return NULL; }
@@ -118,6 +120,8 @@ static alpm_list_t *mount_point_list(alpm_handle_t *handle) fp = fopen("/etc/mnttab", "r");
if(fp == NULL) { + _alpm_log(handle, ALPM_LOG_ERROR, "failed to open file: /etc/mnttab: %s\n", If you use an %s substitution you don't have to introduce another
On Wed, Jun 13, 2012 at 1:00 PM, Dave Reisner <dreisner@archlinux.org> wrote: translatable string. Because you do know these should be all wrapped in _() calls, of course.
+ strerror(errno)); return NULL; }
@@ -153,6 +157,8 @@ static alpm_list_t *mount_point_list(alpm_handle_t *handle) entries = getmntinfo(&fsp, MNT_NOWAIT);
if(entries < 0) { + _alpm_log(handle, ALPM_LOG_ERROR, + "failed to open mount table for reading: %s\n", strerror(errno)); return NULL; }
-- 1.7.10.4
participants (2)
-
Dan McGee
-
Dave Reisner