On Mon, Aug 06, 2012 at 04:03:19PM +0300, Lauri Kasanen wrote:
> Hi
>
> Here's a quick review.
>
> - one gcc warning:
>
> request.c: In function 'request_init':
> request.c:48: warning: missing initializer
> request.c:48: warning: (near initialization for '(anonymous).iov.size')
This warning does not occur on GCC v4.7, but I've changed it so that
each member is initialized.
>
> - why wrap memory functions? That causes a penalty for each call. For
> debugging we have valgrind, without causing production overhead
The wrapping is due to the mk_api. It contains some mem_* functions
and I assume that they should be used. The static variables is then
just a way to remove the dependencies on MKPlugin.h. Storing a
function pointer in a struct or in a static variable should make no
difference, and by assigning it at runtime allows the module to be
reused outside monkey without modification..
It could be a good idea to remove some of the mem_* functions from the
api and use the stdlib directly, as they are just very simple
wrappers. After that, I'd gladly remove the static variables.
>
> - it'd be good to add starting/stopping support so util programs like
> spawn-fcgi aren't needed
This could be a good feature, and I've actually considered using
spawn-fcgi internally, but decided against it quite early.
My reason for doing so is that the feature I like most about fastcgi
is the separation of concerns. The http server handles communication,
routing requests as needed, while the fastcgi server generates
content. Managing fastcgi server processes from monkey would violate
this separation, introduce complexity and also duplicate already
existing functionality in other tools (spawn-fcgi, fcgi-cgi,
php5-fpm).
>
> - I believe assigning structs like that is a GNU extension. Personally I
> don't care, but it might be an issue for some old toolchain
Assigning members by names is a C99 addition but I've no idea about
the portability of {0}. The use of {0} in assignments will be removed.
>
> - Lauri
--
Sonny Karlsson
_______________________________________________
Monkey mailing list
[email protected]
http://lists.monkey-project.com/listinfo/monkey
