2006-08-02: Bob Copeland dixit:
> > 2006-08-02: Bob Copeland dixit:
> > > No, it should be changed to
> > >
> > > if (karma < 0) {
> > > ...
> > > }
> >
> > Why? It is redundant as you already have lk_errors_p("","")
> > before. You may add some text instead of "" if you are not
> > content with the standard lk_errors message...
>
> My mistake, I thought that was the only error path there, but I
> see that there's another if (karma < 0) right after that. I
> suspect it was added because the USB side of the house doesn't
> set an error if "connect" fails.
>
> This whole section of code is a mess:
>
> if (!host) {
> [print some stuff and exit]
> }
> if (host != NULL) { // <- always true, so why test?
>
> Should look more like:
>
> if (!host) {
> lk_errors_p(...);
> usage(0);
> goto cleanup;
> }
> karma = lk_karma_connect(host);
> lk_errors_p(...);
> free(host);
> if (karma < 0)
> goto cleanup;
>
> [do stuff]
> cleanup:
> free(blah);
>
> Gotos are ugly and all that, but this way you don't miss a
> free(). I'll work up a patch if no one gets to it first.
Agree. But there lk_is_karma_mountpoint() and
lk_mountSearch_discover() are a mess too. I think
lk_is_karma_mountpoint() should only be called from (and should
be part of) lk_is_karma_mountpoint.c. lk_mountSearch_discover
is the only one to be called. May be we need to cache the return
value to prevent repeated attempts if it is to be called many
timaes...
I'll see this in detail later this evening.
EV.
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys -- and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
linux-karma-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-karma-devel
