Remove name length limitation from autofs(5). The linear search with
strlens is rather suboptimal, but it's a temporary measure that will be
replaced with red-black trees later on.
Details
Details
Diff Detail
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Comment Actions
I have absolutely no intent of digging into the bug report and debugging the issue to understand what the problem is.
If you do not bother to explain what is the problem and what is the fix, why should I spend time on this ?
Comment Actions
Ah, sorry. Basically, for simplicity autofs used fixed length dirents, and this means there's a name length limit. This patch makes it use variable-length dirents, thus removing the name length limitation.
Comment Actions
Overall it looks fine.
| sys/fs/autofs/autofs_vnops.c | ||
|---|---|---|
| 352 ↗ | (On Diff #14196) | Could the magic '4' constant replaced by __alignof(struct dirent) ? |
| 365 ↗ | (On Diff #14196) | You do not need this memset, you only need to clear the unfilled portion of the memory after the factual end of the string, which is 3 bytes max. |
| 375 ↗ | (On Diff #14196) | Please remove all blank lines from the function (and other places touched by the patch), except the blank line after locals declaration block. Style(9) requests blank line to delimit the scope of the code, explained by previous multi-line comment. |
| 384 ↗ | (On Diff #14196) | The KASSERT is rather useless. |
| 435 ↗ | (On Diff #14196) | What does this check try to achieve ? |
Comment Actions
(I agree with the parts I didn't comment upon, btw.)
| sys/fs/autofs/autofs_vnops.c | ||
|---|---|---|
| 384 ↗ | (On Diff #14196) | It was supposed to quiet down static analysis tools, like Coverity, but on the other hand such a tool should be smart enough to figure out that there can be no error returned here. So ok, I'll remove it. |
| 435 ↗ | (On Diff #14196) | It's to detect whether we should copy out the ".." entry or not, based on the current offset into the directory. |
| sys/fs/autofs/autofs_vnops.c | ||
|---|---|---|
| 454 ↗ | (On Diff #14196) | Yes. The directories are append-only. |
Comment Actions
Remove magic constant, don't zero out things that are overwritten
anyway, remove useless KASSERT, and improve comments.
I've tried removing the whitespace, but it makes things harder
to read for me. Sorry, it's a personal thing; I just prefer to have
more whitespace.
| sys/fs/autofs/autofs_vnops.c | ||
|---|---|---|
| 429 ↗ | (On Diff #14275) | The comment still does not explain the check in if(), it restates what the code does inside the 'then' block. |
Comment Actions
Update comments.
| sys/fs/autofs/autofs_vnops.c | ||
|---|---|---|
| 429 ↗ | (On Diff #14275) | Ok, I've updated the comment on the first block (the one for ".") to mention the offset check, and that it applies to the others. |