On Wed, Apr 6, 2011 at 14:01, Julius Plenz <[email protected]> wrote:
> At some point in the cgit source code, I see constructs like the
> following from ui-repolist.c:32:
>
>    static int get_repo_modtime(const struct cgit_repo *repo, time_t *mtime)
>
> Though *repo is declared `const', you want to modify it, so you place
> a "writable" version on the stack:
>
>    struct cgit_repo *r = (struct cgit_repo *)repo;
>
> IMO this breaks semantics; why not just declare the parameter `struct
> cgit_repo *', so that it's clear the called function may (or indeed
> does) modify it?

Thanks for bringing this up. The preface looks like this:
-cgit learned to sort the repolist by 'last-modified-time' in commit d71c0c725d
-sorting and printing of modtime was unified by commit cbac02c8b0
-caching of modtime was added by commit 8813170390

The end result was that the callback function used by qsort() now
modifies the elements it is sorting to avoid n disk accesses for each
repo, and the obvious fix would be to calculate modtime for all repos
_before_ invoking qsort() (but only when modtime is the sort
criteria).

-- 
larsh

_______________________________________________
cgit mailing list
[email protected]
http://hjemli.net/mailman/listinfo/cgit

Reply via email to