[crossfire] [Crossfire-cvs] SF.net SVN: crossfire: [5296] server

[Andreas Kirschbaum]

Crossfire CVS repository messages. wrote:
> Revision: 5296
>           http://svn.sourceforge.net/crossfire/?rev=5296&view=rev
> Author:   qal21
> Date:     2007-01-04 00:53:58 -0800 (Thu, 04 Jan 2007)
>
> Log Message:
> -----------
> Apply patch #1627442 by Aaron Baugher, to fix bug #1551735. Works by using a key_value of divine_giver_name to objects that are given by a god.
[...]
> Modified: server/trunk/server/gods.c
> ===================================================================
> --- server/trunk/server/gods.c	2007-01-03 21:03:42 UTC (rev 5295)
> +++ server/trunk/server/gods.c	2007-01-04 08:53:58 UTC (rev 5296)
> @@ -207,6 +207,57 @@
>  }
>
>  /**
> + * follower_remove_given_items()

I wonder if it is correct to include the function name here -- wouldn't
it be used as the function (brief) description in the Doxygen output?


[...]
> + * @param pl
> + *     the player object
> + *
> + * @param op
> + *     the object to be searched for items

The implementation allows for NULL pl or op parameters, so this IMO
should be documented (or preferably not checked at all if passing NULL
values is not intended).


[...]
> +        for (tmp = op->inv; tmp != NULL; tmp = next) {
> +            next = tmp->below;   /* backup in case we remove tmp */
> +
> +            const char* given_by = get_ob_key_value(tmp, "divine_giver_name");

Declaring a variable intermixed with statements is not allowed in Ansi C89
(which is to my understanding what the server code should be).


> +            if(given_by == god->name){

This probably will not work: when loading from a file, I think the stored
values will not be shared strings. I.e., this check probably will work until
the player logs out. But after re-login it may fail.


[...]
> +                free_object(tmp);  /* free object */

IMO this is a useless comment and should be removed: the comment does not
contain additional information to what the code says.