You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mynewt.apache.org by GitBox <gi...@apache.org> on 2020/01/24 10:47:21 UTC

[GitHub] [mynewt-core] apache-mynewt-bot commented on issue #2165: FIx: advance_internal should return error instead of using assert

apache-mynewt-bot commented on issue #2165: FIx: advance_internal should return error instead of using assert
URL: https://github.com/apache/mynewt-core/pull/2165#issuecomment-578080533
 
 
   
   <!-- style-bot -->
   
   ## Style check summary
   
   #### encoding/tinycbor/src/cborparser.c
   <details>
   
   ```diff
   @@ -302,7 +313,8 @@
     * point values (SinglePrecisionFloat == Value32Bit and DoublePrecisionFloat ==
     * Value64Bit).
     */
   -uint64_t _cbor_value_decode_int64_internal(const CborValue *value)
   +uint64_t
   +_cbor_value_decode_int64_internal(const CborValue *value)
    {
        uint8_t val = value->parser->d->get8(value->parser->d, value->offset);
    
   @@ -312,11 +324,12 @@
        /* since the additional information can only be Value32Bit or Value64Bit,
         * we just need to test for the one bit those two options differ */
        assert((val & SmallValueMask) == Value32Bit || (val & SmallValueMask) == Value64Bit);
   -    if ((val & 1) == (Value32Bit & 1))
   +    if ((val & 1) == (Value32Bit & 1)) {
            return value->parser->d->get32(value->parser->d, value->offset + 1);
   +    }
    
        assert((val & SmallValueMask) == Value64Bit);
   -        return value->parser->d->get64(value->parser->d, value->offset + 1);
   +    return value->parser->d->get64(value->parser->d, value->offset + 1);
    }
    
    /**
   @@ -329,8 +342,9 @@
     * threads iterating at the same time, but the object can be copied so multiple
     * threads can iterate.
     */
   -CborError cbor_parser_init(struct cbor_decoder_reader *d, int flags,
   -                                CborParser *parser, CborValue *it)
   +CborError
   +cbor_parser_init(struct cbor_decoder_reader *d, int flags,
   +                 CborParser *parser, CborValue *it)
    {
        memset(parser, 0, sizeof(*parser));
        parser->d = d;
   @@ -400,19 +414,23 @@
     *
     * \sa cbor_value_at_end(), cbor_value_advance(), cbor_value_enter_container(), cbor_value_leave_container()
     */
   -CborError cbor_value_advance_fixed(CborValue *it)
   +CborError
   +cbor_value_advance_fixed(CborValue *it)
    {
        assert(it->type != CborInvalidType);
        assert(is_fixed_type(it->type));
   -    if (!it->remaining)
   +    if (!it->remaining) {
            return CborErrorAdvancePastEOF;
   +    }
        return advance_internal(it);
    }
    
   -static CborError advance_recursive(CborValue *it, int nestingLevel)
   -{
   -    if (is_fixed_type(it->type))
   +static CborError
   +advance_recursive(CborValue *it, int nestingLevel)
   +{
   +    if (is_fixed_type(it->type)) {
            return advance_internal(it);
   +    }
    
        if (!cbor_value_is_container(it)) {
            size_t len = SIZE_MAX;
   @@ -420,18 +438,21 @@
        }
    
        /* map or array */
   -    if (nestingLevel == CBOR_PARSER_MAX_RECURSIONS)
   +    if (nestingLevel == CBOR_PARSER_MAX_RECURSIONS) {
            return CborErrorNestingTooDeep;
   +    }
    
        CborError err;
        CborValue recursed;
        err = cbor_value_enter_container(it, &recursed);
   -    if (err)
   +    if (err) {
            return err;
   +    }
        while (!cbor_value_at_end(&recursed)) {
            err = advance_recursive(&recursed, nestingLevel + 1);
   -        if (err)
   +        if (err) {
                return err;
   +        }
        }
        return cbor_value_leave_container(it, &recursed);
    }
   @@ -448,11 +469,13 @@
     *
     * \sa cbor_value_at_end(), cbor_value_advance_fixed(), cbor_value_enter_container(), cbor_value_leave_container()
     */
   -CborError cbor_value_advance(CborValue *it)
   +CborError
   +cbor_value_advance(CborValue *it)
    {
        assert(it->type != CborInvalidType);
   -    if (!it->remaining)
   +    if (!it->remaining) {
            return CborErrorAdvancePastEOF;
   +    }
        return advance_recursive(it, 0);
    }
    
   @@ -484,12 +507,14 @@
     *
     * \sa cbor_value_advance_fixed(), cbor_value_advance()
     */
   -CborError cbor_value_skip_tag(CborValue *it)
   +CborError
   +cbor_value_skip_tag(CborValue *it)
    {
        while (cbor_value_is_tag(it)) {
            CborError err = cbor_value_advance_fixed(it);
   -        if (err)
   +        if (err) {
                return err;
   +        }
        }
        return CborNoError;
    }
   @@ -511,7 +536,8 @@
     *
     * \sa cbor_value_is_container(), cbor_value_leave_container(), cbor_value_advance()
     */
   -CborError cbor_value_enter_container(const CborValue *it, CborValue *recursed)
   +CborError
   +cbor_value_enter_container(const CborValue *it, CborValue *recursed)
    {
        CborError err;
        assert(cbor_value_is_container(it));
   @@ -521,8 +547,9 @@
            recursed->remaining = UINT32_MAX;
            ++recursed->offset;
            err = preparse_value(recursed);
   -        if (err != CborErrorUnexpectedBreak)
   +        if (err != CborErrorUnexpectedBreak) {
                return err;
   +        }
            /* actually, break was expected here
             * it's just an empty container */
            ++recursed->offset;
   @@ -546,8 +573,9 @@
                }
                recursed->remaining *= 2;
            }
   -        if (len != 0)
   +        if (len != 0) {
                return preparse_value(recursed);
   +        }
        }
    
        /* the case of the empty container */
   @@ -568,7 +596,8 @@
     *
     * \sa cbor_value_enter_container(), cbor_value_at_end()
     */
   -CborError cbor_value_leave_container(CborValue *it, const CborValue *recursed)
   +CborError
   +cbor_value_leave_container(CborValue *it, const CborValue *recursed)
    {
        assert(cbor_value_is_container(it));
        assert(recursed->type == CborInvalidType);
   @@ -750,7 +779,8 @@
     *
     * \sa cbor_value_get_type(), cbor_value_is_valid(), cbor_value_is_integer(), cbor_value_get_int64()
     */
   -CborError cbor_value_get_int64_checked(const CborValue *value, int64_t *result)
   +CborError
   +cbor_value_get_int64_checked(const CborValue *value, int64_t *result)
    {
        assert(cbor_value_is_integer(value));
        uint64_t v = _cbor_value_extract_int64_helper(value);
   @@ -765,12 +795,14 @@
         * type).
         */
    
   -    if (unlikely(v > (uint64_t)INT64_MAX))
   +    if (unlikely(v > (uint64_t)INT64_MAX)) {
            return CborErrorDataTooLarge;
   +    }
    
        *result = v;
   -    if (value->flags & CborIteratorFlag_NegativeInteger)
   +    if (value->flags & CborIteratorFlag_NegativeInteger) {
            *result = -*result - 1;
   +    }
        return CborNoError;
    }
    
   @@ -789,7 +821,8 @@
     * \sa cbor_value_get_type(), cbor_value_is_valid(), cbor_value_is_integer(), cbor_value_get_int64(),
     *     cbor_value_get_uint64(), cbor_value_get_int64_checked(), cbor_value_get_raw_integer()
     */
   -CborError cbor_value_get_int_checked(const CborValue *value, int *result)
   +CborError
   +cbor_value_get_int_checked(const CborValue *value, int *result)
    {
        assert(cbor_value_is_integer(value));
        uint64_t v = _cbor_value_extract_int64_helper(value);
   @@ -805,14 +838,16 @@
         */
    
        if (value->flags & CborIteratorFlag_NegativeInteger) {
   -        if (unlikely(v > (unsigned) -(INT_MIN + 1)))
   +        if (unlikely(v > (unsigned) -(INT_MIN + 1))) {
                return CborErrorDataTooLarge;
   +        }
    
            *result = v;
            *result = -*result - 1;
        } else {
   -        if (unlikely(v > (uint64_t)INT_MAX))
   +        if (unlikely(v > (uint64_t)INT_MAX)) {
                return CborErrorDataTooLarge;
   +        }
    
            *result = v;
        }
   @@ -899,7 +934,8 @@
     *
     * \sa cbor_value_get_string_length(), cbor_value_copy_string(), cbor_value_is_length_known()
     */
   -CborError cbor_value_calculate_string_length(const CborValue *value, size_t *len)
   +CborError
   +cbor_value_calculate_string_length(const CborValue *value, size_t *len)
    {
        *len = SIZE_MAX;
        return _cbor_value_copy_string(value, NULL, len, NULL);
   @@ -911,7 +947,8 @@
     * only. */
    typedef uintptr_t (*IterateFunction)(struct cbor_decoder_reader *d, char *dst, int src_offset, size_t len);
    
   -static uintptr_t iterate_noop(struct cbor_decoder_reader *d, char *dst, int src_offset,  size_t len)
   +static uintptr_t
   +iterate_noop(struct cbor_decoder_reader *d, char *dst, int src_offset,  size_t len)
    {
        (void)d;
        (void)dst;
   @@ -920,8 +957,9 @@
        return true;
    }
    
   -static CborError iterate_string_chunks(const CborValue *value, char *buffer, size_t *buflen,
   -                                       bool *result, CborValue *next, IterateFunction func)
   +static CborError
   +iterate_string_chunks(const CborValue *value, char *buffer, size_t *buflen,
   +                      bool *result, CborValue *next, IterateFunction func)
    {
        assert(cbor_value_is_byte_string(value) || cbor_value_is_text_string(value));
    
   @@ -931,14 +969,17 @@
        if (cbor_value_is_length_known(value)) {
            /* easy case: fixed length */
            err = extract_length(value->parser, &offset, &total);
   -        if (err)
   +        if (err) {
                return err;
   -        if (total > (size_t)(value->parser->end - offset))
   +        }
   +        if (total > (size_t)(value->parser->end - offset)) {
                return CborErrorUnexpectedEOF;
   -        if (total <= *buflen)
   +        }
   +        if (total <= *buflen) {
                *result = !!func(value->parser->d, buffer, offset, total);
   -        else
   +        } else {
                *result = false;
   +        }
            offset += total;
        } else {
            /* chunked */
   @@ -950,8 +991,9 @@
                size_t chunkLen;
                size_t newTotal;
    
   -            if (offset == value->parser->end)
   +            if (offset == value->parser->end) {
                    return CborErrorUnexpectedEOF;
   +            }
    
                val = value->parser->d->get8(value->parser->d, offset);
    
   @@ -961,23 +1003,28 @@
                }
    
                /* is this the right type? */
   -            if ((val & MajorTypeMask) != value->type)
   +            if ((val & MajorTypeMask) != value->type) {
                    return CborErrorIllegalType;
   +            }
    
                err = extract_length(value->parser, &offset, &chunkLen);
   -            if (err)
   +            if (err) {
                    return err;
   -
   -            if (unlikely(add_check_overflow(total, chunkLen, &newTotal)))
   +            }
   +
   +            if (unlikely(add_check_overflow(total, chunkLen, &newTotal))) {
                    return CborErrorDataTooLarge;
   -
   -            if (chunkLen > (size_t)(value->parser->end - offset))
   +            }
   +
   +            if (chunkLen > (size_t)(value->parser->end - offset)) {
                    return CborErrorUnexpectedEOF;
   -
   -            if (*result && *buflen >= newTotal)
   +            }
   +
   +            if (*result && *buflen >= newTotal) {
                    *result = !!func(value->parser->d, buffer + total, offset, chunkLen);
   -            else
   +            } else {
                    *result = false;
   +            }
    
                offset += chunkLen;
                total = newTotal;
   @@ -990,7 +1037,7 @@
             * because this is called by function pointer with an abstract
             * reader.  Since this is the output buffer, we can assume that if
             * we have a valid buffer its ok to write a NULL here  */
   -        if(buffer) {
   +        if (buffer) {
                *(buffer + total) = '\0';
            }
        }
   @@ -1069,14 +1116,15 @@
     * \sa cbor_value_dup_text_string(), cbor_value_copy_text_string(), cbor_value_get_string_length(), cbor_value_calculate_string_length()
     */
    
   -CborError _cbor_value_copy_string(const CborValue *value, void *buffer,
   -                                 size_t *buflen, CborValue *next)
   +CborError
   +_cbor_value_copy_string(const CborValue *value, void *buffer,
   +                        size_t *buflen, CborValue *next)
    {
        bool copied_all;
   -    CborError err = iterate_string_chunks(value, (char*)buffer, buflen, &copied_all, next,
   +    CborError err = iterate_string_chunks(value, (char *)buffer, buflen, &copied_all, next,
                                              buffer ? (IterateFunction) value->parser->d->cpy : iterate_noop);
        return err ? err :
   -                 copied_all ? CborNoError : CborErrorOutOfMemory;
   +           copied_all ? CborNoError : CborErrorOutOfMemory;
    }
    
    /**
   @@ -1097,12 +1145,14 @@
     *
     * \sa cbor_value_skip_tag(), cbor_value_copy_text_string()
     */
   -CborError cbor_value_text_string_equals(const CborValue *value, const char *string, bool *result)
   +CborError
   +cbor_value_text_string_equals(const CborValue *value, const char *string, bool *result)
    {
        CborValue copy = *value;
        CborError err = cbor_value_skip_tag(&copy);
   -    if (err)
   +    if (err) {
            return err;
   +    }
        if (!cbor_value_is_text_string(&copy)) {
            *result = false;
            return CborNoError;
   @@ -1185,42 +1235,50 @@
     *
     * \sa cbor_value_is_valid(), cbor_value_text_string_equals(), cbor_value_advance()
     */
   -CborError cbor_value_map_find_value(const CborValue *map, const char *string, CborValue *element)
   +CborError
   +cbor_value_map_find_value(const CborValue *map, const char *string, CborValue *element)
    {
        assert(cbor_value_is_map(map));
        size_t len = strlen(string);
        CborError err = cbor_value_enter_container(map, element);
   -    if (err)
   +    if (err) {
            goto error;
   +    }
    
        while (!cbor_value_at_end(element)) {
            /* find the non-tag so we can compare */
            err = cbor_value_skip_tag(element);
   -        if (err)
   +        if (err) {
                goto error;
   +        }
            if (cbor_value_is_text_string(element)) {
                bool equals;
                size_t dummyLen = len;
                err = iterate_string_chunks(element, CONST_CAST(char *, string), &dummyLen,
                                            &equals, element, map->parser->d->cmp);
   -            if (err)
   +            if (err) {
                    goto error;
   -            if (equals)
   +            }
   +            if (equals) {
                    return preparse_value(element);
   +            }
            } else {
                /* skip this key */
                err = cbor_value_advance(element);
   -            if (err)
   +            if (err) {
                    goto error;
   +            }
            }
    
            /* skip this value */
            err = cbor_value_skip_tag(element);
   -        if (err)
   +        if (err) {
                goto error;
   +        }
            err = cbor_value_advance(element);
   -        if (err)
   +        if (err) {
                goto error;
   +        }
        }
    
        /* not found */
   @@ -1296,7 +1354,8 @@
     *
     * \sa cbor_value_get_type(), cbor_value_is_valid(), cbor_value_is_half_float(), cbor_value_get_float()
     */
   -CborError cbor_value_get_half_float(const CborValue *value, void *result)
   +CborError
   +cbor_value_get_half_float(const CborValue *value, void *result)
    {
        assert(cbor_value_is_half_float(value));
    
   ```
   
   </details>

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services