Hi Anton, On Tue, Jul 11, 2006 at 03:40:08 +0400, Anton wrote:
Hi all, [...] Second - pkgutils have [security] flaw. One package can seriously damage whole system without any warnings. Is conflicts catching should be done in pkgutils or not is arguable, but we have some sort of it already, thus we should make it more consistent. I assume you're refering to #63 here, right? The symlink bug #15 doesn't qualify as a security bug in my opinion. Also, does it really fix #15, i.e. follow symlinks? I might have looked over it too quickly, but I didn't find any indication.
Have you though about other cases which could be potentially annoying to people, e.g. when they manually adjust permissions? Other than that, if the potential "false positives" aren't too bad (analysis needed), it seems like a feature worth having and even adding a couple of lines. I haven't looked at the output it provides in case of a conflict, we should make sure it's clear to the user.
This patch fixes bugs mentioned in subject.
The patch is about the same size as summary sizes of all *.cc files. But! Please, don't be scared, there are really small amount of logic changes. Then it should be a small patch :-)
Most line-consuming changes are: new types, old types typedef'ed, long functions splitting, new debugging code, pkgadd.cc changes regarding indentation and long lines. I'd appreciate if you could split it up. I've reviewed it quickly, and while I like the ideas, some of the refactorings seem to be personal preference: the indentation changes, typedefs etc. and while this doesn't mean that those changes should be rejected, it seems more sensible to look at those changes isolated, and independent from the bug fix. If you could provide individual patches (debug changes, indentation, nesting removal etc.) we can decide on a per patch basis whether we agree there, and eventually come up with a merged version we can all live with (hopefully), and which supports mode-conflicts.
As a final remark, I find your use of typedefs slightly excessive here; I usually prefer to write the real type instead of looking up typedefs, especially as there are so many typedefs with similar names: file_t, files_t, filenames_t, fileconflict_t, fileconflicts_t I haven't really thought a lot about it yet, it just kinda "smells"; maybe some naming changes might help already. Other than that, the patch is very clean, c++ish, and I look forward to discuss the changes in more detail. Thanks for your work so far. Regards, Johannes -- Johannes Winkelmann mailto:jw@smts.ch Zurich, Switzerland http://jw.smts.ch