[pacman-dev] Various code-cleanups

Aaron Griffin aaronmgriffin at gmail.com
Thu Jan 18 11:52:29 EST 2007


On 1/17/07, Dan McGee <dpmcgee at gmail.com> wrote:
> * Removed some unnecessary headers and library links
> * Made things static if possible
> * Cleaned up makefiles a bit
> * Fixed some old comments in the code
> * Fixed some errors the static code checker splint pointed out
> (unnecessary = NULL, bad exit() syntax, casting problems, etc.)
> * Backwards arguments in a memset call in _alpm_db_read (could have been worse)
> * Other various small fixes
>
> Signed-off-by: Dan McGee <dpmcgee at gmail.com>

Merged, with the following changes / comments:

> -       memset(line, 513, 0);
> +       memset(line, 0, 513);
This made me laugh. Feel free to stab me for that one.
> if(!strcmp(filestr, "._install") || !strcmp(filestr, ".INSTALL")) {
As per your comment, I removed the ._install holdover from some older
package version (if there are any around still, I'd be amazed)

> -return _("valid regular expression");
> +return _("invalid regular expression");
Funniest typo ever.

>         assert(list->last != NULL);
I just removed this.  Sure, asserts are nice, but this returns
list->last anyway.  Calling functions can test for validity.

> +/* static function declarations */
> +static pmsyncpkg_t *find_pkginsync(char *needle, pmlist_t *haystack);
> +static int istoonew(pmpkg_t *pkg);
> +static int find_replacements(pmtrans_t *trans, pmdb_t *db_local,
> +                             pmlist_t *dbs_sync);
> +static int pkg_cmp(const void *p1, const void *p2);

I left this out for now.  While it may turn out messy, I'm not a huge
fan of declarations like this for all internal functions, only when it
actually requires a forward-decl.  Still, it's purely cosmetic, so I
can add it back in if you want.

> -                          -ldownload -larchive -lm -lalpm -lssl -lcrypto
> +                          -ldownload -lm -lalpm

Odd.  This may be a build error on my part, but libdownload should
require libssl/libcrypto for https support.


> diff -Naurp pacman-lib.orig/src/pacman/list.c
> pacman-lib.codecleanup/src/pacman/list.c

I left this file untouched.  Reason being that tonight I'm going to
drop it entirely for alpm_list_t (as per another thread).

> +               unsigned int i;
>                 unsigned int cols = getcols();
> -               for(int i=len; i < cols; ++i) {
> +               for(i=len; i < cols; ++i) {

Just FTR: pacman3 should be C99 compliant, so declaring the index
variable outside the for loop shouldn't be required, though again,
it's purely cosmetics at this point 8).

> +static unsigned int maxcols = 80;
Unused.  Removed this.  getcols is used instead.

> +/* TODO breaking abstraction barrier here?
> + *      pacman -> libalpm -> libdownload */
See code for my additions to this comment.


Other than that it's all cool.  Committing now.




More information about the pacman-dev mailing list