darlinghq / darling-dmg

FUSE module for .dmg files (containing an HFS+ filesystem)
http://www.darlinghq.org
GNU General Public License v3.0
278 stars 45 forks source link

Check m_descriptorData size in initFromBuffer() #70

Closed jief666 closed 6 years ago

tomkoen commented 6 years ago

Probably, "else" block is excessive, because initFromBuffer is called when these fields are valid.

jief666 commented 6 years ago

Not in case of the copy constructor, is it ? Copy constructor call operator = that doesn't initialise them. Am I right ?

As a general principle, not taking into account the caller context makes it more reliable.

LubosD commented 6 years ago

@jief666 It'd be better to do the null initialization inside HFSBTreeNode(const HFSBTreeNode& that), then you don't ever need to do it in initFromBuffer().

To put it differently, either keep it in initFromBuffer() and remove it from all constructors or only do it in constructors.

jief666 commented 6 years ago

What about this ? To make it clear, I separated the job : ctors take care of data (m_descriptorData), initFromBuffer take of of convenience pointer derived from data.

jief666 commented 6 years ago

@LubosD, I resolved the conflicts. Note : default ctor, copy ctor and assignment operator are not used in darling-dmg. I didn't remove them is case they are use in darling. I would if you tell me it's ok to remove them.

jief666 commented 6 years ago

Also took the occasion to remove one unused method in HFSCatalogBTree.cpp, and to make findLeafNode returning nullptr instead of an empty/invalid HFSBTreeNode.

jief666 commented 6 years ago

Good, thanks.