HFSAttributeBTree::getattr crashes

[tomkoen]

leafNode = findLeafNode((Key*) &key, cnidAttrComparator);

if findLeafNode returns empty HFSBTreeNode then operator= will crash in initFromBuffer. I'd change the method code to

void initFromBuffer()
{
   if (m_descriptorData.size()) // required check!
   {
      m_descriptor = reinterpret_cast<BTNodeDescriptor*>(&m_descriptorData[0]);
      m_firstRecordOffset = reinterpret_cast<uint16_t*>(descPtr() + m_nodeSize - sizeof(uint16_t));
   }
}

[jief666]

I've just tried and it doesn't crash.

[tomkoen]

Without the check m_descriptorData[0] is trying to access the first element of the vector while the vector is empty.

[jief666]

Yes, I wrote earlier too quick. Seems that vector index out of bounds is undefined. Clang seems to return NULL, which, by coincidence, works well here.

Must be corrected, yes.

Maybe

	HFSBTreeNode& operator=(const HFSBTreeNode& that)
	{
		if (!that.isInvalid()) {
			m_descriptorData = that.m_descriptorData;
			m_nodeSize = that.m_nodeSize;
			initFromBuffer();
		}else{
			m_descriptor = 0;
			m_nodeSize = 0;
			m_firstRecordOffset = 0;
		}
		return *this;
	}

[tomkoen]

Thanks for confirming. Quick code to reproduce:

HFSBTreeNode retEmptyNode() { return HFSBTreeNode(); }

HFSBTreeNode crashNode;
crashNode = retEmptyNode(); // oops

[jief666]

Which platform/compiler do you use ?

[tomkoen]

Windows, Visual Studio 2013

[jief666]

I prefer things not to silently fail. Allowing an index out of bounds, at least in debug mode, is annoying. At least VC crash !

[jief666]

HI @LubosD and @bugaevc.

Should we do a pull request for this, so it's quicker for you ? I'm trained with pull requests, so I can do that :-)

[bugaevc]

Yes please :)

[jief666]

Which fix do you prefer ? In void initFromBuffer() like tomkoen proposed, or in HFSBTreeNode& operator=(const HFSBTreeNode& that)

[jief666]

The one in initBuffer() is better, don't you think ?

[jief666]

Done : #70