Comment 11 for bug 1830865

Revision history for this message
kev (kbackhouse2000) wrote :

Hi Tiago,

Thank you very much for the patch. I am very sorry for not reviewing it sooner. I have a few suggestions:

1. This block of code, which you retained from the original, doesn't make a lot of sense to me:

     if( ( b->dataSize + bytesNeeded ) < INT_MAX )
         new_size = INT_MAX;
     else {
         b->err = BSON_SIZE_OVERFLOW;
         return BSON_ERROR;
     }

   That's going to lead to a 2GB allocation almost every time, which is a bit wasteful.
   I think that entire block of code serves no useful purpose and you should just delete it.

2. Just in case the code is ever compiled for a 32-bit machine, I would recommend adding some bounds checking around the multiplication of new_size by 1.5 (bson.c, line 621).

3. In bson_append_estart, change the type of local variable "len" to size_t.

3. In bson_append_string_base, change the type of parameter "len" to size_t.

4. In bson_append_string_base, change the type of local variable "sl" to size_t.

5. In encoding.c, there are number of parameters and local variables of type int. They should all be changed to size_t.

Thanks,

Kev