[PATCH 11/14] Cleanup: check error return values of buffer handling

Jef Driesen jef at libdivecomputer.org
Wed Jan 3 07:32:23 PST 2018


On 29-12-17 01:35, Dirk Hohndel wrote:
> Coverity CID 207798
> 
> Signed-off-by: Dirk Hohndel <dirk at hohndel.org>
> ---
>   src/divesystem_idive.c | 10 ++++++++--
>   1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/src/divesystem_idive.c b/src/divesystem_idive.c
> index 138aac9fb526..41b8260ff0c4 100644
> --- a/src/divesystem_idive.c
> +++ b/src/divesystem_idive.c
> @@ -491,8 +491,14 @@ divesystem_idive_device_foreach (dc_device_t 
> *abstract, dc_dive_callback_t callb
>   		device_event_emit (abstract, DC_EVENT_PROGRESS, &progress);
>     		(void)dc_buffer_clear(buffer);
> -		dc_buffer_reserve(buffer, commands->header.size + 
> commands->sample.size * nsamples);
> -		dc_buffer_append(buffer, packet, commands->header.size);
> +		if (!dc_buffer_reserve(buffer, commands->header.size + 
> commands->sample.size * nsamples)) {
> +			ERROR (abstract->context, "Insufficient buffer space available.");
> +			return DC_STATUS_NOMEMORY;
> +		}
> +		if (!dc_buffer_append(buffer, packet, commands->header.size)) {
> +			ERROR (abstract->context, "Insufficient buffer space available.");
> +			return DC_STATUS_NOMEMORY;
> +		}
>     		for (unsigned int j = 0; j < nsamples; j += commands->nsamples) {
>   			unsigned int idx = j + 1;
> 

The dc_buffer_reserve() call should indeed be checked, because the 
underlying memory allocation can fail. But for the dc_buffer_append() 
it's a bit pointless. Once the memory is reserved it can't fail anymore 
(unless you pass invalid arguments).

I'm surprised coverity complains about this dc_buffer_append() call, but 
not about the next dc_buffer_append() call a bit further down. If we add 
the error checking, then I think we should do it for both calls.

Jef


More information about the devel mailing list