[pisa-src] Re: r1236 - in trunk/community-operator: Makefile.am co_client.c co_common_packets.h co_server.c
- From: Diego Biurrun <diego@xxxxxxxxxx>
- To: pisa-src@xxxxxxxxxxxxx
- Date: Tue, 20 Oct 2009 13:14:00 +0200
On Mon, Oct 19, 2009 at 08:59:44PM +0200, Jan Marten wrote:
>
> Log:
> Added Community-Operator protocol and renamed functions in co_server.c to
> underscore names
You describe two completely separate things here.
Separate issues should always be committed separately.
> --- trunk/community-operator/Makefile.am Mon Oct 19 17:37:26 2009
> (r1235)
> +++ trunk/community-operator/Makefile.am Mon Oct 19 20:59:44 2009
> (r1236)
> @@ -19,6 +19,7 @@
> INCLUDES += -I@PISA_HIPL_SRCDIR@/libhiptool
> INCLUDES += -I@PISA_HIPL_SRCDIR@/libdht
> +INCLUDES += -I@PISA_HIPL_SRCDIR@/opendht
> INCLUDES += -I@PISA_HIPL_SRCDIR@/hipd
> INCLUDES += -I@PISA_HIPL_SRCDIR@/i3/i3_client
> INCLUDES += -I@PISA_HIPL_SRCDIR@/pjproject/pjnath/include
Why?
This looks unrelated to all the other stuff that is part of this commit.
> --- trunk/community-operator/co_client.c Mon Oct 19 17:37:26 2009
> (r1235)
> +++ trunk/community-operator/co_client.c Mon Oct 19 20:59:44 2009
> (r1236)
> @@ -15,6 +17,10 @@
>
> +static void print_deny_reason(int reason);
Unrelated to this particular commit, but who invented this ugly style of
putting forward declarations of all functions at the top of each C file?
> @@ -94,27 +137,44 @@
> */
> - printf("Receive failed (errno %i): %s\n", errno,
> - strerror(errno));
> + PISA_ERROR("Receive failed (errno %i): %s\n", errno,
> strerror(errno));
> --- /dev/null 00:00:00 1970 (empty, because file is newly added)
> +++ trunk/community-operator/co_common_packets.h Mon Oct 19 20:59:44
> 2009 (r1236)
> @@ -0,0 +1,59 @@
> +
> +typedef struct co_certificate_request
> +{
> +} __attribute__ ((packed)) co_certificate_request;
> +
> +typedef struct co_certificate_deny
> +{
> +} __attribute__ ((packed)) co_certificate_deny;
> +
> +typedef struct co_certificate
> +{
> +} __attribute__ ((packed)) co_certificate;
> +
> +typedef struct co_error
> +{
> +} __attribute__ ((packed)) co_error;
> +
> +typedef struct co_packet
> +{
> +} __attribute__ ((packed)) co_packet;
Also unrelated to this particular commit, but why are (almost) all the
structs in PISA marked as packed?
> --- trunk/community-operator/co_server.c Mon Oct 19 17:37:26 2009
> (r1235)
> +++ trunk/community-operator/co_server.c Mon Oct 19 20:59:44 2009
> (r1236)
> @@ -7,37 +7,52 @@
>
> - printf("socket() failed.\n");
> + PISA_ERROR("socket() failed.\n");
>
> - printf("setsockopt failed with SO_REUSEADDR.\n");
> + PISA_ERROR("setsockopt failed with SO_REUSEADDR.\n");
>
> @@ -46,16 +61,37 @@
>
> - printf("bind failed.\n");
> + PISA_ERROR("bind failed.\n");
>
> - printf("socket created and bound to port %i\n", port);
> + PISA_INFO("socket created and bound to port %i\n", port);
> - printf("sendto failed.\n");
> + PISA_ERROR("sendto failed.\n");
>
> - printf("sent bytes (%i) do not match expected value (%i).\n",
> - sent, bytes);
> - printf("the request has been answered.\n");
> + PISA_ERROR("sent bytes (%i) do not match expected value
> (%i).\n", sent,
> + sizeof(co_packet));
> + PISA_INFO("the request has been answered.\n");
>
> - printf("You're not root. HIPD won't sign certificates.\n");
> + PISA_ERROR("You're not root. HIPD won't sign certificates.\n");
>
> - printf("community operator server ready\n");
> + PISA_INFO("community operator server ready\n");
This should have been a separate commit ..
> @@ -7,37 +7,52 @@
> -static int createServerSocket(void)
> +static int create_server_socket(void)
>
> @@ -64,13 +100,17 @@
> -static char *getCertificateForHit(struct in6_addr *hit)
> +static char *get_certificate_for_hit(struct in6_addr *hit, int *reason)
>
> @@ -78,110 +118,223 @@
>
> -static void answerCertificateQuery(struct sockaddr_in6 *cl_addr)
> +static int answer_certificate_query(struct sockaddr_in6 *cl_addr)
> - sock = createServerSocket();
> + sock = create_server_socket();
>
.. as should this.
> @@ -78,110 +118,223 @@
>
> - if (!(conn = pisa_hitlist_find(allowed, hit)))
> + if (!(conn = pisa_hitlist_find(allowed, hit))) {
> + *reason = DENY_HIT_NOT_ALLOWED;
> return NULL;
> - if (conn->expiration < not_before)
> + }
> + if (conn->expiration < not_before) {
> + *reason = DENY_HIT_EXPIRED;
> return NULL;
> - if (conn->expiration < not_after)
> + }
> + if (conn->expiration < not_after) {
> not_after = conn->expiration;
> + }
It seems that this mostly adds useless curly braces. The addition of
the curly braces mixed in with everything else does make the diff hard
to read though.
> + if (users != NULL) {
if (users)
Diego
Other related posts:
