[patch iwhd] Factor out Microhttpd response
Jim Meyering
jim at meyering.net
Wed Aug 10 16:01:50 UTC 2011
Pete Zaitcev wrote:
> This patch fell out from the authentication work: if one needs something
> new done in every place a response is formed (such as to supply a header
> for authentication), then we have too many places to do it. However, the
> code seems more straightforward with it alone. Shorter, too.
>
> ---
> rest.c | 316 +++++++++++++++----------------------------------------
> 1 file changed, 87 insertions(+), 229 deletions(-)
Very nice clean-up.
Not often you see a no-semantic-change patch deleting so many lines.
Two nits below.
> diff --git a/rest.c b/rest.c
> index 7bf8fe1..8c5b630 100644
> --- a/rest.c
> +++ b/rest.c
> @@ -106,6 +106,8 @@ typedef struct {
> MHD_AccessHandlerCallback handler;
> } rule;
>
> +static void simple_closer (void *ctx);
> +
> static unsigned short my_port = MY_PORT;
> static LIST_HEAD(gc_list, _my_state) my_states; /* this keeps GC informed */
> static pthread_mutex_t my_lock = PTHREAD_MUTEX_INITIALIZER;
> @@ -168,6 +170,45 @@ validate_url (const char *url)
> return !is_reserved(slash+1,reserved_name);
> }
>
> +static int do_resp(struct MHD_Connection *conn, my_state *ms,
> + unsigned int status_code, const char *etag,
> + const char *datastr)
> +{
> + struct MHD_Response *resp;
> + size_t datalen = datastr? strlen(datastr): 0;
> +
> + resp = MHD_create_response_from_data(datalen,(void *)datastr,
> + MHD_NO,MHD_NO);
> + if (!resp) {
> + return MHD_NO;
> + }
> +
> + if (etag) {
> + MHD_add_response_header(resp,"ETag",etag);
> + }
> +
> + MHD_queue_response(conn,status_code,resp);
> + MHD_destroy_response(resp);
> + return MHD_YES;
> +}
> +
> +static int do_resp_from_cb(struct MHD_Connection *conn, my_state *ms,
> + ssize_t (*func)(void *ctx, uint64_t pos, char *buf, size_t max))
> +{
> + struct MHD_Response *resp;
> +
> + resp = MHD_create_response_from_callback(MHD_SIZE_UNKNOWN,
> + CB_BLOCK_SIZE, func, ms, simple_closer);
> + if (!resp) {
> + log_msg(0,"MHD_crfc failed");
> + simple_closer(ms);
> + return MHD_NO;
> + }
> + MHD_queue_response(conn,MHD_HTTP_OK,resp);
> + MHD_destroy_response(resp);
> + return MHD_YES;
> +}
...
> @@ -2272,15 +2137,8 @@ access_handler_0 (void *cctx, struct MHD_Connection *conn, const char *url,
> return MHD_NO;
> }
>
> - log_msg(0,"bad request m=%s u=%s",method,url);
> -
> - resp = MHD_create_response_from_data(0,NULL,MHD_NO,MHD_NO);
> - if (!resp) {
> - return MHD_NO;
> - }
> - MHD_queue_response(conn,MHD_HTTP_NOT_FOUND,resp);
> - MHD_destroy_response(resp);
> - return MHD_YES;
> + log_msg(0,"bad request m=%s u=%s\n",method,url);
> + return do_resp(conn,ms,MHD_HTTP_NOT_FOUND,NULL,NULL);
> }
>
> static int
Please adjust as follows:
- put the function name in first column, for consistency in rest.c.
It's also good because it helps diff-like tools determine the
name of the function in which a change occurs.
- don't add the trailing "\n" in the log_msg format string.
I.e., just apply this patch:
diff --git a/rest.c b/rest.c
index 8c5b630..34b0590 100644
--- a/rest.c
+++ b/rest.c
@@ -170,7 +170,8 @@ validate_url (const char *url)
return !is_reserved(slash+1,reserved_name);
}
-static int do_resp(struct MHD_Connection *conn, my_state *ms,
+static int
+do_resp(struct MHD_Connection *conn, my_state *ms,
unsigned int status_code, const char *etag,
const char *datastr)
{
@@ -192,7 +193,8 @@ static int do_resp(struct MHD_Connection *conn, my_state *ms,
return MHD_YES;
}
-static int do_resp_from_cb(struct MHD_Connection *conn, my_state *ms,
+static int
+do_resp_from_cb(struct MHD_Connection *conn, my_state *ms,
ssize_t (*func)(void *ctx, uint64_t pos, char *buf, size_t max))
{
struct MHD_Response *resp;
@@ -2137,7 +2139,7 @@ access_handler_0 (void *cctx, struct MHD_Connection *conn, const char *url,
return MHD_NO;
}
- log_msg(0,"bad request m=%s u=%s\n",method,url);
+ log_msg(0,"bad request m=%s u=%s",method,url);
return do_resp(conn,ms,MHD_HTTP_NOT_FOUND,NULL,NULL);
}
More information about the iwhd-devel
mailing list