[pacman-dev] [PATCH] Check return value of chdir and getwd

[Allan McRae]

On 30/06/10 23:38, Dan McGee wrote:
> Typo in patch title ^^^, we use getcwd
>
> On Sun, Jun 27, 2010 at 5:40 AM, Allan McRae<allan at archlinux.org>  wrote:
>> Prevents compiler warnings when building with -D_FORTIFY_SOURCE=2
>>
>> Signed-off-by: Allan McRae<allan at archlinux.org>
>> ---
>>
>> In commit_single_pkg in lib/libalpm/add.c the failure of changing directory has now become
>> an error.  From the comment above it, it looks like it should be an error if it fails, but
>> I have been wrong before...
> Yes, it probably should be, given the bit about hardlink extraction.
>
>
>>   lib/libalpm/add.c   |   18 +++++++++++++-----
>>   lib/libalpm/util.c  |    8 ++++++--
>>   src/pacman/pacman.c |   17 +++++++++++++++--
>>   3 files changed, 34 insertions(+), 9 deletions(-)
>>
>> diff --git a/lib/libalpm/add.c b/lib/libalpm/add.c
>> index f39a0ec..e45073e 100644
>> --- a/lib/libalpm/add.c
>> +++ b/lib/libalpm/add.c
>> @@ -556,6 +556,7 @@ static int commit_single_pkg(pmpkg_t *newpkg, int pkg_current, int pkg_count,
>>                 struct archive *archive;
>>                 struct archive_entry *entry;
>>                 char cwd[PATH_MAX] = "";
>> +               int restore_cwd = 0;
>>                 _alpm_log(PM_LOG_DEBUG, "extracting files\n");
>>
>> @@ -579,11 +580,16 @@ static int commit_single_pkg(pmpkg_t *newpkg, int pkg_current, int pkg_count,
>>                 /* save the cwd so we can restore it later */
>>                 if(getcwd(cwd, PATH_MAX) == NULL) {
>>                         _alpm_log(PM_LOG_ERROR, _("could not get current working directory\n"));
>> -                       cwd[0] = 0;
>> +               } else {
>> +                       restore_cwd = 1;
>>                 }
>>
>>                 /* libarchive requires this for extracting hard links */
>> -               chdir(handle->root);
>> +               if(chdir(handle->root) != 0) {
>> +                       _alpm_log(PM_LOG_ERROR, _("could not change directory to %s (%s)\n"), handle->root, strerror(errno));
>> +                       ret = -1;
>> +                       goto cleanup;
>> +               }
>>
>>                 /* call PROGRESS once with 0 percent, as we sort-of skip that here */
>>                 if(is_upgrade) {
>> @@ -629,9 +635,11 @@ static int commit_single_pkg(pmpkg_t *newpkg, int pkg_current, int pkg_count,
>>                 }
>>                 archive_read_finish(archive);
>>
>> -               /* restore the old cwd is we have it */
>> -               if(strlen(cwd)) {
>> -                       chdir(cwd);
>> +               /* restore the old cwd if we have it */
>> +               if(restore_cwd) {
>> +                       if(chdir(cwd) != 0) {
> Since there is no branching in the conditional, I'd probably prefer
> if(restore_cwd&&  chdir(cwd) != 0) {
>

<snip>

All is fixed as suggested on my working branch.

Allan