Can the audio block release() method not deal gracefully with NULL pointers?

[MarkT]

Currently there is a commented out piece of code at the start of release()
that I feel would be useful to uncomment:

// Release ownership of a data block.  If no
// other streams have ownership, the block is
// returned to the free pool
void AudioStream::release(audio_block_t *block)
{
	//if (block == NULL) return;
	uint32_t mask = (0x80000000 >> (31 - (block->memory_pool_index & 0x1F)));
	uint32_t index = block->memory_pool_index >> 5;

	__disable_irq();
	if (block->ref_count > 1) {
		block->ref_count--;
	} else {
		//Serial.print("reles:");
		//Serial.println((uint32_t)block, HEX);
		memory_pool_available_mask[index] |= mask;
		if (index < memory_pool_first_mask) memory_pool_first_mask = index;
		memory_used--;
	}
	__enable_irq();
}

Passing on the requirement to check for NULL to the clients of this call just makes for complicated boiler-plate
code in various update() functions throughout the audio library, and this is a route for obscure pattern-sensitive
bugs to creep in. Currently getting this wrong crashes the system too which is not so friendly to debug.

In many places NULL for an audio block is taken to be equivalent to a block of all zeros, so I think the basic
operations should be happy to handle a NULL audio_block_t pointer.

For example I'd like to be able to code like this:

void AudioWhatever::update(void)
{
  audio_block_t  b0 = receiveReadonly (0) ;
  audio_block_t  b1 = receiveReadWrite (1) ;
  audio_block_t  b2 = receiveReadonly (2) ;
  audio_block_t b3 = allocate() ;
  if (b0 == NULL or b1 == NULL or b2 == NULL or b3 == NULL)
  {
    release (b0) ; release (b1) ; release (b2) ; release (b3) ;
    return ;
  }
  ....
  ....
}

And I'd make the same comment about the transmit() method, it shouldn't crash the system if you
happen to pass NULL to it, but simply retun gracefully.

 

[h4yn0nnym0u5e]

Already done for release()* in the Dynamic Audio Objects library I've been working on. You're right, transmit() should likewise be immunised. Incoming...

*Actually, I made it so releasing ANY block address not from the audio block pool does nothing. This allows the use of statically-defined blocks: there are several instances where it's handy to have a genuine "silent block" available, and AudioStream objects make their own and then have to treat them carefully. I have yet to go through and remove all such, and make them work with a single global AudioStream::silentBlock.