[pacman-dev] Missing sanity checks for path -> weird behaviors and segfaults

[Xavier]

On Sun, Jul 22, 2007 at 11:06:44PM +0200, Xavier wrote:
> Also, if no Cache Dirs is set, pacman will try to fallback to /tmp/ , but
> then it segfaults. (try pacman -Sw bar).
> 
> Apparently it segfaults on this in libalpm/sync.c , sync_commit function,
> after downloading the file succesfully :
> 718   for(i = handle->dbs_sync; i; i = i->next)
> 
> I found it weird that inside that loop, i was used a second time in an inner
> loop :
> 758       for(i = alpm_option_get_cachedirs(); i; i = alpm_list_next(i)) {
> 
> I thought that was the problem, but it didn't fix the segfault.
> 
> Second strange thing is that the files list seem to be freed in any cases :
>  795       FREELIST(files);
> 
> but it's used at the end of that _alpm_sync_commit function :
> 1018   if(!validcache && !(trans->flags & PM_TRANS_FLAG_DOWNLOADONLY)) {
> 1019     /* delete packages */
> 1020     for(i = files; i; i = i->next) {
> 1021       unlink(i->data);
> 1022     }
> 1023   }
> 
> Finally, if the cache isn't valid (so it fallbacks to /tmp/) and DOWNLOADONLY is used,
> files won't be deleted, but will they be used next times?
> Because the fallback (lines 779-790) happen after the check of existence of
> the package in the cache (lines 740-744).
> I can't check what happens because of the segfault.

The segfault still happened because this loop is there twice,
but I only saw one occurence of it :
for(i = alpm_option_get_cachedirs(); i; i = alpm_list_next(i)) {

And both times, it overlaps with the main loop.
I guess Dan was tired when he wrote this ;)

Anyway, I've been looking back at this code because of bug 6404 :
http://bugs.archlinux.org/task/6404#comment17750

I've two questions :

1) is it very important to delete the packages when they are put in /tmp ?
(the current code doing that is totally bogus as pointed out above,
 but I wonder if I couldn't just remove it instead of fixing it)

2) If I want to factor the "looking for package in cache"
and "get a valide cache dir" codes, where would such functions belong ?
I put them in libalpm/server.c , is this ok ?