On Sun, Oct 18, 2009 at 5:02 AM, Xavier <shiningxc@gmail.com> wrote:
On Sun, Oct 18, 2009 at 6:03 AM, Laszlo Papp <djszapi2@gmail.com> wrote:
* Size examined str* function usage is a common coding practice, because it's more safer to avoid breakage while using str* functions.
Signed-off-by: Laszlo Papp <djszapi@archlinux.us> --- lib/libalpm/add.c | 4 ++-- lib/libalpm/be_files.c | 10 +++++----- lib/libalpm/conflict.c | 2 +- lib/libalpm/db.c | 4 ++-- lib/libalpm/trans.c | 2 +- lib/libalpm/util.c | 16 +++++++++------- src/pacman/sync.c | 2 +- src/pacman/util.c | 2 +- 8 files changed, 22 insertions(+), 20 deletions(-)
diff --git a/lib/libalpm/add.c b/lib/libalpm/add.c index ebcd6a5..209c38e 100644 --- a/lib/libalpm/add.c +++ b/lib/libalpm/add.c @@ -311,7 +311,7 @@ static int extract_single_file(struct archive *archive, size_t backup_len = strlen(oldbackup) + 34; MALLOC(backup, backup_len, RET_ERR(PM_ERR_MEMORY, -1));
- sprintf(backup, "%s\t%s", oldbackup, hash_pkg); + snprintf(backup, backup_len, "%s\t%s", oldbackup, hash_pkg);
I don't think this provides us any extra safety, because we already compute exactly the size of the destination and allocate that memory.
I think it makes more sense where we define a fixed array, like char path[PATH_MAX], and we then copy strings of different size to it : src/pacman/pacman.c: snprintf(path, PATH_MAX, "%s%s", alpm_option_get_root(), DBPATH + 1); src/pacman/pacman.c: snprintf(path, PATH_MAX, "%s%s", alpm_option_get_root(), LOGFILE + 1);
There is zero added safety to this. All of the patches that have been coming in are relatively bogus. We've done strlen() in nearly all cases anyway. The "n" family of operations also silently truncates. This is probably worse off for us, as it could lead to silent failures. -Dan