Forum Rule: Always post complete source code & details to reproduce any issue!
Results 1 to 2 of 2

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

  1. #1
    Senior Member
    Join Date
    Jul 2020
    Posts
    1,450

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

    Currently there is a commented out piece of code at the start of release()
    that I feel would be useful to uncomment:
    Code:
    // 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:
    Code:
    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.

  2. #2
    Senior Member
    Join Date
    Apr 2021
    Location
    Cambridgeshire, UK
    Posts
    243
    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.
    Last edited by h4yn0nnym0u5e; 11-26-2021 at 10:30 PM. Reason: Add info

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •