Message ID | 333e896cf5bcadd8547fbe4a06388dd3104ff910.camel@hammerspace.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [GIT,PULL] Please pull NFS changes for Linux 5.3 | expand |
On Thu, Jul 18, 2019 at 1:25 PM Trond Myklebust <trondmy@hammerspace.com> wrote: > > git://git.linux-nfs.org/projects/trondmy/linux-nfs.git tags/nfs-for-5.3-1 This got a conflict with the debugfs "don't behave differently on failures" changes in net/sunrpc/debugfs.c. See commit 0a0762c6c604 ("sunrpc: no need to check return value of debugfs_create functions") by Greg, but I suspect you were already aware of this. I did a hack-and-slash "remove the error handling", and the end result looks sane. Except I left the "if the snprintf overflows" error handling in place, even if nothing then cares about the returned error. I think my merge resolution makes sense, but I thought I'd mention it in case you had something else in mind. Honestly, the snprintf() checks in do_xprt_debugfs() look kind o fpointless, but the comment is also wrong: char link[9]; /* enough for 8 hex digits + NULL */ that comment was copied from the "name[]" array in rpc_clnt_debugfs_register(), but it's bogus, since you actually use len = snprintf(link, sizeof(link), "xprt%d", *nump); on the thing. And you know what? If you have so many links that "xprt%d" doesn't fit in 8 chars plus NUL, maybe you don't really care? But it's also worth noting that the whole snprintf() overflow check is *wrong* to begin with. When you do if (len > sizeof(link)) return -1; you're testing the wrong thing entirely. The returned "len" is the length that would have been printed _without_ the ending NUL character, so you actually had a truncation even if it returns "sizeof(link)" - because then the NUL character was written instead of the last character. So the overflow test *should* have been if (len >= sizeof(link)) return -1; but I suspect the correct thing to do is to just say "we don't care" and remove that error check entirely. Same goes for the other case ("len > sizeof(name)"). At some point error handling doesn't actually add value, as long as the error itself isn't fatal. And when the error handling itself is wrong, it's doubly suspect. But as mentioned, I did *not* remove this part of the error handling. I only removed the debugfs parts. The error handling may be wrong, but it is what it is, and it doesn't really matter. Linus
The pull request you sent on Thu, 18 Jul 2019 20:25:03 +0000:
> git://git.linux-nfs.org/projects/trondmy/linux-nfs.git tags/nfs-for-5.3-1
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/6860c981b9672324cb53b883cfda8d2ea1445ff1
Thank you!