[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