fix: off-by-one error in convertPathnameToDirName
#4454
Conversation
| /* get dir name from pathname similar to 'dirname()' */ | ||
| assert(pathname != NULL); | ||
|
|
||
| /* remove trailing '/' chars */ |
There was a problem hiding this comment.
To be fair, I'm not sure if this part of the code makes sense.
The goal of the method convertPathnameToDirName() is to convert a full path of a filename into a just its directory part.
But if pathname ends with a / character, then it already represents a directory name.
In which case, there should be no need to change anything.
In contrast, if we remove the last /, the next stage would strip the last part of the path, and we would end up with the upper directory. And I suspect that's not the wanted behavior.
At this point, my suspicion is that this paragraph is useless, and should just be removed. "Fixing" it could actually introduce problems.
Note: a good follow up could be to improve the tests, by finding scenarios that introduce problems if this function is badly defined. Since our CI signal is all-greens, that means it does not capture the issue.
3 participants
Fixes #4380
pathname[len-1]instead ofpathname[len]to compare/modify last character instead of null terminatorlen == 0check to inside loop to avoid OOB