On Fri, Oct 05, 2012 at 03:55:39AM +0200, Michael Hanselmann wrote: > - LoadAllowedFileStoragePaths: Loads a list of allowed file storage > paths from a file > - CheckFileStoragePath: Checks a path against the list of allowed paths > > The unit test for “utils.IsBelowDir” is updated with cases which weren't > tested before.
LGTM, one comment: > +def CheckFileStoragePath(path, _filename=pathutils.FILE_STORAGE_PATHS_FILE): > + """Checks if a path is allowed for file storage. > + > + @type path: string > + @param path: Path to check > + @raise errors.FileStoragePathError: If the path is not allowed > + > + """ > + allowed = LoadAllowedFileStoragePaths(_filename) > + > + return _CheckFileStoragePath(path, allowed) When reading the patch, this return was confusing, since _CheckFile… doesn't have an explicit return value. thanks, iustin
