Sam James <sam@xxxxxxxxxx> writes: > Christoph Hellwig <hch@xxxxxxxxxxxxx> writes: > >> On Fri, Dec 15, 2023 at 01:36:41AM +0000, Sam James wrote: >>> + /* We're only interested in supporting an off_t which can handle >=4GiB. */ >> >> This adds a < 80 character line. Also I find the wording a bit odd, the >> point is that xfsprogs relies on (it or rather will with your entire >> series), so maybe: >> >> /* >> * xfsprogs relies on the LFS interfaces with a 64-bit off_t to >> * actually support sensible file systems sizes. >> */ >> >> And while I'm nitpicking, maybe a better place would be to move this to >> libxfs as that's where we really care. If you use the C99 static_assert >> instead of the kernel BUILD_BUG_ON this can even move outside a function >> and just into a header somewhere, say include/xfs,h. Which actually >> happens to have this assert in an awkware open coded way already: >> >> /* >> * make sure that any user of the xfs headers has a 64bit off_t type >> */ >> extern int xfs_assert_largefile[sizeof(off_t)-8]; >> >> Enough of my stream of consciousness, sorry. To summarize the findings: >> >> - we don't really need this patch all >> - but cleaning up xfs_assert_largefile to just use static_assert would >> probably be nice to have anyway > > Thanks, I agree, but I think static_assert is C11 (and then it gets a > nicer name in C23). If it's still fine for us, I can then use it. > > Does it change your thinking at all or should I send a v4 with it > included? ping. I don't mind doing a followup, but I'd love to get this in given there's a bunch of other projects still to handle with this sort of problem. > > Thanks, > sam
