On Fri, Jun 3, 2011 at 6:49 PM, Allan McRae <al...@archlinux.org> wrote: > On 04/06/11 08:40, Dan McGee wrote: >> >> This keeps duplicate code to a minimum and also addresses the issue >> where calling set_cachedirs() didn't canonicalize the passed-in paths. >> This will come in more handy as we refactor some of these option setters >> away. >> >> Signed-off-by: Dan McGee<d...@archlinux.org> >> --- > > <snip> > >> >> int SYMEXPORT alpm_option_set_cachedirs(alpm_list_t *cachedirs) >> { >> + alpm_list_t *i, *new_cachedirs = NULL; >> ASSERT(handle != NULL, RET_ERR(PM_ERR_HANDLE_NULL, -1)); >> + for(i = cachedirs; i; i = i->next) { >> + int ret = alpm_option_add_cachedir(i->data); >> + if(ret) { >> + return ret; >> + } >> + } >> if(handle->cachedirs) FREELIST(handle->cachedirs); >> - handle->cachedirs = cachedirs; >> + handle->cachedirs = new_cachedirs; >> + FREELIST(cachedirs); >> return 0; >> } >> > > Should the clean-up done to this function in PATCH 3/6 be in here instead? > This confused the hell out of me until I saw the next patch and I'm not > sure how the clean-up fits into the next patch.
This was a tough one to split, so I think it would be a tad confusing either way. It might make more sense to have this bit above be a patch all on its own; I can attempt to do that. -Dan