cleaned up parser code
authorMartin Willi <martin@strongswan.org>
Mon, 18 May 2009 11:12:52 +0000 (13:12 +0200)
committerMartin Willi <martin@strongswan.org>
Mon, 18 May 2009 11:13:12 +0000 (13:13 +0200)
src/charon/encoding/parser.c

index 05b35b6..04d4cca 100644 (file)
@@ -86,29 +86,52 @@ struct private_parser_t {
 };
 
 /**
+ * Forward declaration
+ */
+static status_t parse_payload(private_parser_t *this,
+                                                         payload_type_t payload_type, payload_t **payload);
+
+/**
+ * Log invalid length error
+ */
+static bool short_input(private_parser_t *this, int number)
+{
+       DBG1(DBG_ENC, "  not enough input to parse rule %d %N",
+                number, encoding_type_names, this->rules[number].type);
+       return FALSE;
+}
+
+/**
+ * Log unaligned rules
+ */
+static bool bad_bitpos(private_parser_t *this, int number)
+{
+       DBG1(DBG_ENC, "  found rule %d %N on bitpos %d",
+                number, encoding_type_names, this->rules[number].type, this->bit_pos);
+       return FALSE;
+}
+
+/**
  * Parse a 4-Bit unsigned integer from the current parsing position.
  */
-static status_t parse_uint4(private_parser_t *this, int rule_number, u_int8_t *output_pos)
+static bool parse_uint4(private_parser_t *this, int rule_number,
+                                               u_int8_t *output_pos)
 {
-       if (this->byte_pos + sizeof(u_int8_t)  > this->input_roof)
+       if (this->byte_pos + sizeof(u_int8_t) > this->input_roof)
        {
-               DBG1(DBG_ENC, "  not enough input to parse rule %d %N",
-                        rule_number, encoding_type_names, this->rules[rule_number].type);
-               return PARSE_ERROR;
+               return short_input(this, rule_number);
        }
        switch (this->bit_pos)
        {
                case 0:
-                       /* caller interested in result ? */
-                       if (output_pos != NULL)
+                       if (output_pos)
                        {
                                *output_pos = *(this->byte_pos) >> 4;
                        }
                        this->bit_pos = 4;
                        break;
                case 4:
-                       /* caller interested in result ? */
-                       if (output_pos != NULL)
+                       if (output_pos)
                        {
                                *output_pos = *(this->byte_pos) & 0x0F;
                        }
@@ -116,221 +139,175 @@ static status_t parse_uint4(private_parser_t *this, int rule_number, u_int8_t *o
                        this->byte_pos++;
                        break;
                default:
-                       DBG2(DBG_ENC, "  found rule %d %N on bitpos %d",
-                                rule_number, encoding_type_names,
-                                this->rules[rule_number].type, this->bit_pos);
-                       return PARSE_ERROR;
+                       return bad_bitpos(this, rule_number);
        }
-       
-       if (output_pos != NULL)
+       if (output_pos)
        {
                DBG3(DBG_ENC, "   => %d", *output_pos);
        }
-       
-       return SUCCESS;
+       return TRUE;
 }
 
 /**
  * Parse a 8-Bit unsigned integer from the current parsing position.
  */
-static status_t parse_uint8(private_parser_t *this, int rule_number, u_int8_t *output_pos)
+static bool parse_uint8(private_parser_t *this, int rule_number,
+                                               u_int8_t *output_pos)
 {
-       if (this->byte_pos + sizeof(u_int8_t)  > this->input_roof)
+       if (this->byte_pos + sizeof(u_int8_t) > this->input_roof)
        {
-               DBG1(DBG_ENC, "  not enough input to parse rule %d %N",
-                        rule_number, encoding_type_names, this->rules[rule_number].type);
-               return PARSE_ERROR;
+               return short_input(this, rule_number);
        }
        if (this->bit_pos)
        {
-               DBG1(DBG_ENC, "  found rule %d %N on bitpos %d",
-                        rule_number, encoding_type_names,
-                        this->rules[rule_number].type, this->bit_pos);
-               return PARSE_ERROR;
+               return bad_bitpos(this, rule_number);
        }
-
-       /* caller interested in result ? */
-       if (output_pos != NULL)
+       if (output_pos)
        {
                *output_pos = *(this->byte_pos);
                DBG3(DBG_ENC, "   => %d", *output_pos);
        }
        this->byte_pos++;
-       
-       return SUCCESS;
+       return TRUE;
 }
 
 /**
  * Parse a 15-Bit unsigned integer from the current parsing position.
  */
-static status_t parse_uint15(private_parser_t *this, int rule_number, u_int16_t *output_pos)
+static bool parse_uint15(private_parser_t *this, int rule_number,
+                                                u_int16_t *output_pos)
 {
        if (this->byte_pos + sizeof(u_int16_t) > this->input_roof)
        {
-               DBG1(DBG_ENC, "  not enough input to parse rule %d %N",
-                        rule_number, encoding_type_names, this->rules[rule_number].type);
-               return PARSE_ERROR;
+               return short_input(this, rule_number);
        }
        if (this->bit_pos != 1)
        {
-               DBG2(DBG_ENC, "  found rule %d %N on bitpos %d", rule_number,
-                        encoding_type_names, this->rules[rule_number].type, this->bit_pos);
-               return PARSE_ERROR;
+               return bad_bitpos(this, rule_number);
        }
-       /* caller interested in result ? */
-       if (output_pos != NULL)
+       if (output_pos)
        {
                *output_pos = ntohs(*((u_int16_t*)this->byte_pos)) & ~0x8000;
                DBG3(DBG_ENC, "   => %d", *output_pos);
        }
        this->byte_pos += 2;
        this->bit_pos = 0;
-       
-       return SUCCESS;
+       return TRUE;
 }
 
 /**
  * Parse a 16-Bit unsigned integer from the current parsing position.
  */
-static status_t parse_uint16(private_parser_t *this, int rule_number, u_int16_t *output_pos)
+static bool parse_uint16(private_parser_t *this, int rule_number,
+                                                u_int16_t *output_pos)
 {
        if (this->byte_pos + sizeof(u_int16_t) > this->input_roof)
        {
-               DBG1(DBG_ENC, "  not enough input to parse rule %d %N",
-                        rule_number, encoding_type_names, this->rules[rule_number].type);
-               return PARSE_ERROR;
+               return short_input(this, rule_number);
        }
        if (this->bit_pos)
        {
-               DBG1(DBG_ENC, "  found rule %d %N on bitpos %d", rule_number,
-                        encoding_type_names, this->rules[rule_number].type, this->bit_pos);
-               return PARSE_ERROR;
+               return bad_bitpos(this, rule_number);
        }
-       /* caller interested in result ? */
-       if (output_pos != NULL)
+       if (output_pos)
        {
                *output_pos = ntohs(*((u_int16_t*)this->byte_pos));
-               
                DBG3(DBG_ENC, "   => %d", *output_pos);
        }
        this->byte_pos += 2;
-       
-       return SUCCESS;
+       return TRUE;
 }
 /**
  * Parse a 32-Bit unsigned integer from the current parsing position.
  */
-static status_t parse_uint32(private_parser_t *this, int rule_number, u_int32_t *output_pos)
+static bool parse_uint32(private_parser_t *this, int rule_number,
+                                                u_int32_t *output_pos)
 {
        if (this->byte_pos + sizeof(u_int32_t) > this->input_roof)
        {
-               DBG1(DBG_ENC, "  not enough input to parse rule %d %N",
-                        rule_number, encoding_type_names, this->rules[rule_number].type);
-               return PARSE_ERROR;
+               return short_input(this, rule_number);
        }
        if (this->bit_pos)
        {
-               DBG1(DBG_ENC, "  found rule %d %N on bitpos %d", rule_number,
-                        encoding_type_names, this->rules[rule_number].type, this->bit_pos);
-               return PARSE_ERROR;
+               return bad_bitpos(this, rule_number);
        }
-       /* caller interested in result ? */
-       if (output_pos != NULL)
+       if (output_pos)
        {
                *output_pos = ntohl(*((u_int32_t*)this->byte_pos));
-               
                DBG3(DBG_ENC, "   => %d", *output_pos);
        }
        this->byte_pos += 4;
-       
-       return SUCCESS;
+       return TRUE;
 }
 
 /**
  * Parse a 64-Bit unsigned integer from the current parsing position.
  */
-static status_t parse_uint64(private_parser_t *this, int rule_number, u_int64_t *output_pos)
+static bool parse_uint64(private_parser_t *this, int rule_number,
+                                                        u_int64_t *output_pos)
 {
        if (this->byte_pos + sizeof(u_int64_t) > this->input_roof)
        {
-               DBG1(DBG_ENC, "  not enough input to parse rule %d %N",
-                        rule_number, encoding_type_names, this->rules[rule_number].type);
-               return PARSE_ERROR;
+               return short_input(this, rule_number);
        }
        if (this->bit_pos)
        {
-               DBG1(DBG_ENC, "  found rule %d %N on bitpos %d", rule_number,
-                        encoding_type_names, this->rules[rule_number].type, this->bit_pos);
-               return PARSE_ERROR;
+               return bad_bitpos(this, rule_number);
        }
-       /* caller interested in result ? */
-       if (output_pos != NULL)
+       if (output_pos)
        {
                /* assuming little endian host order */
                *(output_pos + 1) = ntohl(*((u_int32_t*)this->byte_pos));
                *output_pos = ntohl(*(((u_int32_t*)this->byte_pos) + 1));
-               
-               DBG3(DBG_ENC, "   => %b", (void*)output_pos, sizeof(u_int64_t));
+               DBG3(DBG_ENC, "   => %b", output_pos, sizeof(u_int64_t));
        }
        this->byte_pos += 8;
-
-       return SUCCESS;
+       return TRUE;
 }
 
 /**
  * Parse a given amount of bytes and writes them to a specific location
  */
-static status_t parse_bytes (private_parser_t *this, int rule_number, u_int8_t *output_pos,size_t bytes)
+static bool parse_bytes (private_parser_t *this, int rule_number,
+                                                u_int8_t *output_pos, size_t bytes)
 {
        if (this->byte_pos + bytes > this->input_roof)
        {
-               DBG1(DBG_ENC, "  not enough input to parse rule %d %N",
-                        rule_number, encoding_type_names, this->rules[rule_number].type);
-               return PARSE_ERROR;
+               return short_input(this, rule_number);
        }
        if (this->bit_pos)
        {
-               DBG1(DBG_ENC, "  found rule %d %N on bitpos %d", rule_number,
-                        encoding_type_names, this->rules[rule_number].type, this->bit_pos);
-               return PARSE_ERROR;
+               return bad_bitpos(this, rule_number);
        }
-
-       /* caller interested in result ? */
-       if (output_pos != NULL)
+       if (output_pos)
        {
                memcpy(output_pos,this->byte_pos,bytes);
-               
-               DBG3(DBG_ENC, "   => %b", (void*)output_pos, bytes);
+               DBG3(DBG_ENC, "   => %b", output_pos, bytes);
        }
        this->byte_pos += bytes;
-       
-       return SUCCESS;
+       return TRUE;
 }
 
 /**
  * Parse a single Bit from the current parsing position
  */
-static status_t parse_bit(private_parser_t *this, int rule_number, bool *output_pos)
+static bool parse_bit(private_parser_t *this, int rule_number,
+                                         bool *output_pos)
 {
        if (this->byte_pos + sizeof(u_int8_t) > this->input_roof)
        {
-               DBG1(DBG_ENC, "  not enough input to parse rule %d %N",
-                        rule_number, encoding_type_names, this->rules[rule_number].type);
-               return PARSE_ERROR;
+               return short_input(this, rule_number);
        }
-       /* caller interested in result ? */
-       if (output_pos != NULL)
+       if (output_pos)
        {       
                u_int8_t mask;
                mask = 0x01 << (7 - this->bit_pos);
                *output_pos = *this->byte_pos & mask;
                
                if (*output_pos)
-               {
-                       /* set to a "clean", comparable true */
+               {       /* set to a "clean", comparable true */
                        *output_pos = TRUE;
                }
-               
                DBG3(DBG_ENC, "   => %d", *output_pos);
        }
        this->bit_pos = (this->bit_pos + 1) % 8;
@@ -338,85 +315,75 @@ static status_t parse_bit(private_parser_t *this, int rule_number, bool *output_
        {
                this->byte_pos++;
        }
-       
-       return SUCCESS;
+       return TRUE;
 }
 
 /**
  * Parse substructures in a list.
  */
-static status_t parse_list(private_parser_t *this, int rule_number, linked_list_t **output_pos, payload_type_t payload_type, size_t length)
+static bool parse_list(private_parser_t *this, int rule_number,
+               linked_list_t **output_pos, payload_type_t payload_type, size_t length)
 {
-       linked_list_t * list = *output_pos;
+       linked_list_t *list = *output_pos;
        
        if (length < 0)
        {
-               DBG1(DBG_ENC, "  invalid length for rule %d %N",
-                        rule_number, encoding_type_names, this->rules[rule_number].type);
-               return PARSE_ERROR;
+               return short_input(this, rule_number);
        }
-       
        if (this->bit_pos)
        {
-               DBG1(DBG_ENC, "  found rule %d %N on bitpos %d", rule_number,
-                        encoding_type_names, this->rules[rule_number].type, this->bit_pos);
-               return PARSE_ERROR;
+               return bad_bitpos(this, rule_number);
        }
-       
        while (length > 0)
        {
                u_int8_t *pos_before = this->byte_pos;
                payload_t *payload;
-               status_t status;
+               
                DBG2(DBG_ENC, "  %d bytes left, parsing recursively %N",
                         length, payload_type_names, payload_type);
-               status = this->public.parse_payload((parser_t*)this, payload_type, &payload);
-               if (status != SUCCESS)
+               
+               if (parse_payload(this, payload_type, &payload) != SUCCESS)
                {
                        DBG1(DBG_ENC, "  parsing of a %N substructure failed",
                                 payload_type_names, payload_type);
-                       return status;
+                       return FALSE;
                }
                list->insert_last(list, payload);
                length -= this->byte_pos - pos_before;
        }
        *output_pos = list;
-       return SUCCESS;
+       return TRUE;
 }
 
 /**
  * Parse data from current parsing position in a chunk.
  */
-static status_t parse_chunk(private_parser_t *this, int rule_number, chunk_t *output_pos, size_t length)
+static bool parse_chunk(private_parser_t *this, int rule_number,
+                                                       chunk_t *output_pos, size_t length)
 {
        if (this->byte_pos + length > this->input_roof)
        {
-               DBG1(DBG_ENC, "  not enough input (%d bytes) to parse rule %d %N",
-                        length, rule_number, encoding_type_names, this->rules[rule_number].type);
-               return PARSE_ERROR;
+               return short_input(this, rule_number);
        }
        if (this->bit_pos)
        {
-               DBG1(DBG_ENC, "  found rule %d %N on bitpos %d", rule_number,
-                        encoding_type_names, this->rules[rule_number].type, this->bit_pos);
-               return PARSE_ERROR;
+               return bad_bitpos(this, rule_number);
        }
-       if (output_pos != NULL)
+       if (output_pos)
        {
-               output_pos->len = length;
-               output_pos->ptr = malloc(length);
+               *output_pos = chunk_alloc(length);
                memcpy(output_pos->ptr, this->byte_pos, length);
+               DBG3(DBG_ENC, "   => %b", output_pos->ptr, length);
        }
        this->byte_pos += length;
-       DBG3(DBG_ENC, "   => %b", (void*)output_pos->ptr, length);
-       
-       return SUCCESS;
+       return TRUE;
 }
 
 /**
  * Implementation of parser_t.parse_payload.
  */
-static status_t parse_payload(private_parser_t *this, payload_type_t payload_type, payload_t **payload)
+static status_t parse_payload(private_parser_t *this,
+                                                         payload_type_t payload_type, payload_t **payload)
 {
        payload_t *pld;
        void *output;
@@ -455,7 +422,7 @@ static status_t parse_payload(private_parser_t *this, payload_type_t payload_typ
                {
                        case U_INT_4:
                        {
-                               if (parse_uint4(this, rule_number, output + rule->offset) != SUCCESS)
+                               if (!parse_uint4(this, rule_number, output + rule->offset))
                                {
                                        pld->destroy(pld);
                                        return PARSE_ERROR;
@@ -464,7 +431,7 @@ static status_t parse_payload(private_parser_t *this, payload_type_t payload_typ
                        }
                        case U_INT_8:
                        {
-                               if (parse_uint8(this, rule_number, output + rule->offset) != SUCCESS)
+                               if (!parse_uint8(this, rule_number, output + rule->offset))
                                {
                                        pld->destroy(pld);
                                        return PARSE_ERROR;
@@ -473,7 +440,7 @@ static status_t parse_payload(private_parser_t *this, payload_type_t payload_typ
                        }
                        case U_INT_16:
                        {
-                               if (parse_uint16(this, rule_number, output + rule->offset) != SUCCESS)
+                               if (!parse_uint16(this, rule_number, output + rule->offset))
                                {
                                        pld->destroy(pld);
                                        return PARSE_ERROR;
@@ -482,7 +449,7 @@ static status_t parse_payload(private_parser_t *this, payload_type_t payload_typ
                        }
                        case U_INT_32:
                        {
-                               if (parse_uint32(this, rule_number, output + rule->offset) != SUCCESS)
+                               if (!parse_uint32(this, rule_number, output + rule->offset))
                                {
                                        pld->destroy(pld);
                                        return PARSE_ERROR;
@@ -491,7 +458,7 @@ static status_t parse_payload(private_parser_t *this, payload_type_t payload_typ
                        }
                        case U_INT_64:
                        {
-                               if (parse_uint64(this, rule_number, output + rule->offset) != SUCCESS)
+                               if (!parse_uint64(this, rule_number, output + rule->offset))
                                {
                                        pld->destroy(pld);
                                        return PARSE_ERROR;
@@ -500,7 +467,7 @@ static status_t parse_payload(private_parser_t *this, payload_type_t payload_typ
                        }
                        case IKE_SPI:
                        {
-                               if (parse_bytes(this, rule_number, output + rule->offset,8) != SUCCESS)
+                               if (!parse_bytes(this, rule_number, output + rule->offset, 8))
                                {
                                        pld->destroy(pld);
                                        return PARSE_ERROR;
@@ -509,7 +476,7 @@ static status_t parse_payload(private_parser_t *this, payload_type_t payload_typ
                        }
                        case RESERVED_BIT:
                        {
-                               if (parse_bit(this, rule_number, NULL) != SUCCESS)
+                               if (!parse_bit(this, rule_number, NULL))
                                {
                                        pld->destroy(pld);
                                        return PARSE_ERROR;
@@ -518,7 +485,7 @@ static status_t parse_payload(private_parser_t *this, payload_type_t payload_typ
                        }
                        case RESERVED_BYTE:
                        {
-                               if (parse_uint8(this, rule_number, NULL) != SUCCESS)
+                               if (!parse_uint8(this, rule_number, NULL))
                                {
                                        pld->destroy(pld);
                                        return PARSE_ERROR;
@@ -527,7 +494,7 @@ static status_t parse_payload(private_parser_t *this, payload_type_t payload_typ
                        }
                        case FLAG:
                        {
-                               if (parse_bit(this, rule_number, output + rule->offset) != SUCCESS)
+                               if (!parse_bit(this, rule_number, output + rule->offset))
                                {
                                        pld->destroy(pld);
                                        return PARSE_ERROR;
@@ -536,7 +503,7 @@ static status_t parse_payload(private_parser_t *this, payload_type_t payload_typ
                        }
                        case PAYLOAD_LENGTH:
                        {
-                               if (parse_uint16(this, rule_number, output + rule->offset) != SUCCESS)
+                               if (!parse_uint16(this, rule_number, output + rule->offset))
                                {
                                        pld->destroy(pld);
                                        return PARSE_ERROR;
@@ -551,7 +518,7 @@ static status_t parse_payload(private_parser_t *this, payload_type_t payload_typ
                        }
                        case HEADER_LENGTH:
                        {
-                               if (parse_uint32(this, rule_number, output + rule->offset) != SUCCESS)
+                               if (!parse_uint32(this, rule_number, output + rule->offset))
                                {
                                        pld->destroy(pld);
                                        return PARSE_ERROR;
@@ -560,7 +527,7 @@ static status_t parse_payload(private_parser_t *this, payload_type_t payload_typ
                        }
                        case SPI_SIZE:
                        {
-                               if (parse_uint8(this, rule_number, output + rule->offset) != SUCCESS)
+                               if (!parse_uint8(this, rule_number, output + rule->offset))
                                {
                                        pld->destroy(pld);
                                        return PARSE_ERROR;
@@ -570,7 +537,8 @@ static status_t parse_payload(private_parser_t *this, payload_type_t payload_typ
                        }
                        case SPI:
                        {
-                               if (parse_chunk(this, rule_number, output + rule->offset, spi_size) != SUCCESS)
+                               if (!parse_chunk(this, rule_number, output + rule->offset,
+                                                                spi_size))
                                {
                                        pld->destroy(pld);
                                        return PARSE_ERROR;
@@ -580,8 +548,9 @@ static status_t parse_payload(private_parser_t *this, payload_type_t payload_typ
                        case PROPOSALS:
                        {
                                if (payload_length < SA_PAYLOAD_HEADER_LENGTH ||
-                                       parse_list(this, rule_number, output + rule->offset, PROPOSAL_SUBSTRUCTURE,
-                                               payload_length - SA_PAYLOAD_HEADER_LENGTH) != SUCCESS)
+                                       !parse_list(this, rule_number, output + rule->offset,
+                                                               PROPOSAL_SUBSTRUCTURE,
+                                                               payload_length - SA_PAYLOAD_HEADER_LENGTH))
                                {
                                        pld->destroy(pld);
                                        return PARSE_ERROR;
@@ -590,9 +559,11 @@ static status_t parse_payload(private_parser_t *this, payload_type_t payload_typ
                        }
                        case TRANSFORMS:
                        {
-                               if (payload_length < spi_size + PROPOSAL_SUBSTRUCTURE_HEADER_LENGTH ||
-                                       parse_list(this, rule_number, output + rule->offset, TRANSFORM_SUBSTRUCTURE,
-                                               payload_length - spi_size - PROPOSAL_SUBSTRUCTURE_HEADER_LENGTH) != SUCCESS)
+                               if (payload_length <
+                                                       spi_size + PROPOSAL_SUBSTRUCTURE_HEADER_LENGTH ||
+                                       !parse_list(this, rule_number, output + rule->offset,
+                                                       TRANSFORM_SUBSTRUCTURE, payload_length - spi_size -
+                                                                               PROPOSAL_SUBSTRUCTURE_HEADER_LENGTH))
                                {
                                        pld->destroy(pld);
                                        return PARSE_ERROR;
@@ -602,8 +573,9 @@ static status_t parse_payload(private_parser_t *this, payload_type_t payload_typ
                        case TRANSFORM_ATTRIBUTES:
                        {
                                if (payload_length < TRANSFORM_SUBSTRUCTURE_HEADER_LENGTH ||
-                                       parse_list(this, rule_number, output + rule->offset, TRANSFORM_ATTRIBUTE,
-                                               payload_length - TRANSFORM_SUBSTRUCTURE_HEADER_LENGTH) != SUCCESS)
+                                       !parse_list(this, rule_number, output + rule->offset,
+                                               TRANSFORM_ATTRIBUTE,
+                                               payload_length - TRANSFORM_SUBSTRUCTURE_HEADER_LENGTH))
                                {
                                        pld->destroy(pld);
                                        return PARSE_ERROR;
@@ -613,8 +585,9 @@ static status_t parse_payload(private_parser_t *this, payload_type_t payload_typ
                        case CONFIGURATION_ATTRIBUTES:
                        {
                                if (payload_length < CP_PAYLOAD_HEADER_LENGTH ||
-                                       parse_list(this, rule_number, output + rule->offset, CONFIGURATION_ATTRIBUTE,
-                                               payload_length - CP_PAYLOAD_HEADER_LENGTH) != SUCCESS)
+                                       !parse_list(this, rule_number, output + rule->offset,
+                                                               CONFIGURATION_ATTRIBUTE,
+                                                               payload_length - CP_PAYLOAD_HEADER_LENGTH))
                                {
                                        pld->destroy(pld);
                                        return PARSE_ERROR;
@@ -623,7 +596,7 @@ static status_t parse_payload(private_parser_t *this, payload_type_t payload_typ
                        }
                        case ATTRIBUTE_FORMAT:
                        {
-                               if (parse_bit(this, rule_number, output + rule->offset) != SUCCESS)
+                               if (!parse_bit(this, rule_number, output + rule->offset))
                                {
                                        pld->destroy(pld);
                                        return PARSE_ERROR;
@@ -633,7 +606,7 @@ static status_t parse_payload(private_parser_t *this, payload_type_t payload_typ
                        }
                        case ATTRIBUTE_TYPE:
                        {
-                               if (parse_uint15(this, rule_number, output + rule->offset) != SUCCESS)
+                               if (!parse_uint15(this, rule_number, output + rule->offset))
                                {
                                        pld->destroy(pld);
                                        return PARSE_ERROR;
@@ -643,7 +616,7 @@ static status_t parse_payload(private_parser_t *this, payload_type_t payload_typ
                        }
                        case CONFIGURATION_ATTRIBUTE_LENGTH:
                        {
-                               if (parse_uint16(this, rule_number, output + rule->offset) != SUCCESS)
+                               if (!parse_uint16(this, rule_number, output + rule->offset))
                                {
                                        pld->destroy(pld);
                                        return PARSE_ERROR;
@@ -653,7 +626,7 @@ static status_t parse_payload(private_parser_t *this, payload_type_t payload_typ
                        }
                        case ATTRIBUTE_LENGTH_OR_VALUE:
                        {
-                               if (parse_uint16(this, rule_number, output + rule->offset) != SUCCESS)
+                               if (!parse_uint16(this, rule_number, output + rule->offset))
                                {
                                        pld->destroy(pld);
                                        return PARSE_ERROR;
@@ -663,21 +636,20 @@ static status_t parse_payload(private_parser_t *this, payload_type_t payload_typ
                        }
                        case ATTRIBUTE_VALUE:
                        {
-                               if (attribute_format == FALSE)
+                               if (attribute_format == FALSE &&
+                                       !parse_chunk(this, rule_number, output + rule->offset,
+                                                                attribute_length))
                                {
-                                       if (parse_chunk(this, rule_number, output + rule->offset, attribute_length) != SUCCESS)
-                                       {
-                                               pld->destroy(pld);
-                                               return PARSE_ERROR;
-                                       }
+                                       pld->destroy(pld);
+                                       return PARSE_ERROR;
                                }
                                break;
                        }
                        case NONCE_DATA:
                        {
                                if (payload_length < NONCE_PAYLOAD_HEADER_LENGTH ||
-                                       parse_chunk(this, rule_number, output + rule->offset,
-                                               payload_length - NONCE_PAYLOAD_HEADER_LENGTH) != SUCCESS)
+                                       !parse_chunk(this, rule_number, output + rule->offset,
+                                                                payload_length - NONCE_PAYLOAD_HEADER_LENGTH))
                                {
                                        pld->destroy(pld);
                                        return PARSE_ERROR;
@@ -687,8 +659,8 @@ static status_t parse_payload(private_parser_t *this, payload_type_t payload_typ
                        case ID_DATA:
                        {
                                if (payload_length < ID_PAYLOAD_HEADER_LENGTH ||
-                                       parse_chunk(this, rule_number, output + rule->offset,
-                                               payload_length - ID_PAYLOAD_HEADER_LENGTH) != SUCCESS)
+                                       !parse_chunk(this, rule_number, output + rule->offset,
+                                                                payload_length - ID_PAYLOAD_HEADER_LENGTH))
                                {
                                        pld->destroy(pld);
                                        return PARSE_ERROR;
@@ -698,8 +670,8 @@ static status_t parse_payload(private_parser_t *this, payload_type_t payload_typ
                        case AUTH_DATA:
                        {
                                if (payload_length < AUTH_PAYLOAD_HEADER_LENGTH ||
-                                       parse_chunk(this, rule_number, output + rule->offset,
-                                               payload_length - AUTH_PAYLOAD_HEADER_LENGTH) != SUCCESS)
+                                       !parse_chunk(this, rule_number, output + rule->offset,
+                                                                payload_length - AUTH_PAYLOAD_HEADER_LENGTH))
                                {
                                        pld->destroy(pld);
                                        return PARSE_ERROR;
@@ -709,8 +681,8 @@ static status_t parse_payload(private_parser_t *this, payload_type_t payload_typ
                        case CERT_DATA:
                        {
                                if (payload_length < CERT_PAYLOAD_HEADER_LENGTH ||
-                                       parse_chunk(this, rule_number, output + rule->offset,
-                                               payload_length - CERT_PAYLOAD_HEADER_LENGTH) != SUCCESS)
+                                       !parse_chunk(this, rule_number, output + rule->offset,
+                                                                payload_length - CERT_PAYLOAD_HEADER_LENGTH))
                                {
                                        pld->destroy(pld);
                                        return PARSE_ERROR;
@@ -720,8 +692,8 @@ static status_t parse_payload(private_parser_t *this, payload_type_t payload_typ
                        case CERTREQ_DATA:
                        {
                                if (payload_length < CERTREQ_PAYLOAD_HEADER_LENGTH ||
-                                       parse_chunk(this, rule_number, output + rule->offset,
-                                               payload_length - CERTREQ_PAYLOAD_HEADER_LENGTH) != SUCCESS)
+                                       !parse_chunk(this, rule_number, output + rule->offset,
+                                                                payload_length - CERTREQ_PAYLOAD_HEADER_LENGTH))
                                {
                                        pld->destroy(pld);
                                        return PARSE_ERROR;
@@ -731,8 +703,8 @@ static status_t parse_payload(private_parser_t *this, payload_type_t payload_typ
                        case EAP_DATA:
                        {
                                if (payload_length < EAP_PAYLOAD_HEADER_LENGTH ||
-                                       parse_chunk(this, rule_number, output + rule->offset,
-                                               payload_length - EAP_PAYLOAD_HEADER_LENGTH) != SUCCESS)
+                                       !parse_chunk(this, rule_number, output + rule->offset,
+                                                                payload_length - EAP_PAYLOAD_HEADER_LENGTH))
                                {
                                        pld->destroy(pld);
                                        return PARSE_ERROR;
@@ -742,8 +714,8 @@ static status_t parse_payload(private_parser_t *this, payload_type_t payload_typ
                        case SPIS:
                        {
                                if (payload_length < DELETE_PAYLOAD_HEADER_LENGTH ||
-                                       parse_chunk(this, rule_number, output + rule->offset,
-                                               payload_length - DELETE_PAYLOAD_HEADER_LENGTH) != SUCCESS)
+                                       !parse_chunk(this, rule_number, output + rule->offset,
+                                                                payload_length - DELETE_PAYLOAD_HEADER_LENGTH))
                                {
                                        pld->destroy(pld);
                                        return PARSE_ERROR;
@@ -753,8 +725,8 @@ static status_t parse_payload(private_parser_t *this, payload_type_t payload_typ
                        case VID_DATA:
                        {
                                if (payload_length < VENDOR_ID_PAYLOAD_HEADER_LENGTH ||
-                                       parse_chunk(this, rule_number, output + rule->offset,
-                                               payload_length - VENDOR_ID_PAYLOAD_HEADER_LENGTH) != SUCCESS)
+                                       !parse_chunk(this, rule_number, output + rule->offset,
+                                                       payload_length - VENDOR_ID_PAYLOAD_HEADER_LENGTH))
                                {
                                        pld->destroy(pld);
                                        return PARSE_ERROR;
@@ -763,8 +735,8 @@ static status_t parse_payload(private_parser_t *this, payload_type_t payload_typ
                        }
                        case CONFIGURATION_ATTRIBUTE_VALUE:
                        {
-                               size_t data_length = attribute_length;
-                               if (parse_chunk(this, rule_number, output + rule->offset, data_length) != SUCCESS)
+                               if (!parse_chunk(this, rule_number, output + rule->offset,
+                                                                attribute_length))
                                {
                                        pld->destroy(pld);
                                        return PARSE_ERROR;
@@ -774,8 +746,8 @@ static status_t parse_payload(private_parser_t *this, payload_type_t payload_typ
                        case KEY_EXCHANGE_DATA:
                        {
                                if (payload_length < KE_PAYLOAD_HEADER_LENGTH ||
-                                       parse_chunk(this, rule_number, output + rule->offset,
-                                               payload_length - KE_PAYLOAD_HEADER_LENGTH) != SUCCESS)
+                                       !parse_chunk(this, rule_number, output + rule->offset,
+                                                                payload_length - KE_PAYLOAD_HEADER_LENGTH))
                                {
                                        pld->destroy(pld);
                                        return PARSE_ERROR;
@@ -785,8 +757,8 @@ static status_t parse_payload(private_parser_t *this, payload_type_t payload_typ
                        case NOTIFICATION_DATA:
                        {
                                if (payload_length < NOTIFY_PAYLOAD_HEADER_LENGTH + spi_size ||
-                                       parse_chunk(this, rule_number, output + rule->offset,
-                                               payload_length - NOTIFY_PAYLOAD_HEADER_LENGTH - spi_size) != SUCCESS)
+                                       !parse_chunk(this, rule_number, output + rule->offset,
+                                               payload_length - NOTIFY_PAYLOAD_HEADER_LENGTH - spi_size))
                                {
                                        pld->destroy(pld);
                                        return PARSE_ERROR;
@@ -796,8 +768,8 @@ static status_t parse_payload(private_parser_t *this, payload_type_t payload_typ
                        case ENCRYPTED_DATA:
                        {
                                if (payload_length < ENCRYPTION_PAYLOAD_HEADER_LENGTH ||
-                                       parse_chunk(this, rule_number, output + rule->offset,
-                                               payload_length - ENCRYPTION_PAYLOAD_HEADER_LENGTH) != SUCCESS)
+                                       !parse_chunk(this, rule_number, output + rule->offset,
+                                                       payload_length - ENCRYPTION_PAYLOAD_HEADER_LENGTH))
                                {
                                        pld->destroy(pld);
                                        return PARSE_ERROR;
@@ -806,7 +778,7 @@ static status_t parse_payload(private_parser_t *this, payload_type_t payload_typ
                        }
                        case TS_TYPE:
                        {
-                               if (parse_uint8(this, rule_number, output + rule->offset) != SUCCESS)
+                               if (!parse_uint8(this, rule_number, output + rule->offset))
                                {
                                        pld->destroy(pld);
                                        return PARSE_ERROR;
@@ -817,7 +789,9 @@ static status_t parse_payload(private_parser_t *this, payload_type_t payload_typ
                        case ADDRESS:
                        {
                                size_t address_length = (ts_type == TS_IPV4_ADDR_RANGE) ? 4 : 16;
-                               if (parse_chunk(this, rule_number, output + rule->offset,address_length) != SUCCESS)
+                               
+                               if (!parse_chunk(this, rule_number, output + rule->offset,
+                                                                address_length))
                                {
                                        pld->destroy(pld);
                                        return PARSE_ERROR;
@@ -827,8 +801,9 @@ static status_t parse_payload(private_parser_t *this, payload_type_t payload_typ
                        case TRAFFIC_SELECTORS:
                        {
                                if (payload_length < TS_PAYLOAD_HEADER_LENGTH ||
-                                       parse_list(this, rule_number, output + rule->offset, TRAFFIC_SELECTOR_SUBSTRUCTURE,
-                                               payload_length - TS_PAYLOAD_HEADER_LENGTH) != SUCCESS)
+                                       !parse_list(this, rule_number, output + rule->offset,
+                                                               TRAFFIC_SELECTOR_SUBSTRUCTURE,
+                                                               payload_length - TS_PAYLOAD_HEADER_LENGTH))
                                {
                                        pld->destroy(pld);
                                        return PARSE_ERROR;
@@ -838,8 +813,8 @@ static status_t parse_payload(private_parser_t *this, payload_type_t payload_typ
                        case UNKNOWN_DATA:
                        {
                                if (payload_length < UNKNOWN_PAYLOAD_HEADER_LENGTH ||
-                                       parse_chunk(this, rule_number, output + rule->offset,
-                                               payload_length - UNKNOWN_PAYLOAD_HEADER_LENGTH) != SUCCESS)
+                                       !parse_chunk(this, rule_number, output + rule->offset,
+                                                               payload_length - UNKNOWN_PAYLOAD_HEADER_LENGTH))
                                {
                                        pld->destroy(pld);
                                        return PARSE_ERROR;
@@ -869,8 +844,7 @@ static status_t parse_payload(private_parser_t *this, payload_type_t payload_typ
  */
 static int get_remaining_byte_count (private_parser_t *this)
 {
-       int count = (this->input_roof - this->byte_pos);
-       return count;
+       return this->input_roof - this->byte_pos;
 }
 
 /**
@@ -897,7 +871,7 @@ parser_t *parser_create(chunk_t data)
 {
        private_parser_t *this = malloc_thing(private_parser_t);
        
-       this->public.parse_payload = (status_t(*)(parser_t*,payload_type_t,payload_t**)) parse_payload;
+       this->public.parse_payload = (status_t(*)(parser_t*,payload_type_t,payload_t**))parse_payload;
        this->public.reset_context = (void(*)(parser_t*)) reset_context;
        this->public.get_remaining_byte_count = (int (*) (parser_t *))get_remaining_byte_count;
        this->public.destroy = (void(*)(parser_t*)) destroy;
@@ -907,6 +881,6 @@ parser_t *parser_create(chunk_t data)
        this->bit_pos = 0;
        this->input_roof = data.ptr + data.len;
        
-       return (parser_t*)this;
+       return &this->public;
 }