Date: Sun, 23 Feb 2003 22:04:10 -0800 From: Terry Lambert <tlambert2@mindspring.com> To: Bruce Evans <bde@zeta.org.au> Cc: David Syphers <dsyphers@uchicago.edu>, Kirk McKusick <mckusick@beastie.mckusick.com>, freebsd-current@FreeBSD.ORG Subject: Re: BOOT2_UFS=UFS1_ONLY works for today's current Message-ID: <3E59B5DA.66EDBC85@mindspring.com> References: <20030224144742.L5105-100000@gamplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Bruce Evans wrote: > > Personally, I think the changes should be #ifdef'ed the current > > version of GCC; when GCC rev's, hopefully its 64 bit operations > > handling will have improved. > > This would wrong, since ufs2 depends on the changes to actually work > for file systems larger than about 1TB. If it's not going to compile correctly, it's not going to work. What we are bitching about here is that someone wants to use 64 bit operations that GCC can't handle. Waving our hands isn't going to make the problem go away, and adding code that doesn't compile correctly with the current compiler won't, either. > Blaming gcc's 64-bit operations seems to be wrong anyway. gcc actually > has good handling of (32, 32) -> 64 bit operations which are the only > types involved here in at least fs.h, boot2 still fits for me when it > is compiled the previous version of gcc. It has 19 bytes to spare: So we're agreed: it's a GCC issue, which is resolved by downgrading the compiler, or by upgrading it, after the new version is fixed (which is the direction I suggested going). > % -#define cgbase(fs, c) ((ufs2_daddr_t)((fs)->fs_fpg * (c))) > % +#define cgbase(fs, c) (((ufs2_daddr_t)(fs)->fs_fpg) * (c)) > % #define cgdmin(fs, c) (cgstart(fs, c) + (fs)->fs_dblkno) /* 1st data */ > > This changes a (32, 32) -> 32 bit (possibly overflowing) operation to a > (32, 32) -> 64 bit (never overflowing) operation. The 1386 hardware > already does the wider operation so all gcc has to do is not discard the > high 32-bits, which it does reasonably well. So it is agreed that this is good? [ ... rest of patches == "style bugs" ... ] > All of these changes add style bugs IMO. Except for the change to > cgbase(), they add redundant parentheses around "(cast)(foo)->bar", > but properly parenthesized macros are hard enough to read with only > non-redundant parentheses. The change to cgbase() moves parentheses > so that they are redundant instead of just wrong. OK... so what should be done? Just the cgbase() change? Does this fix the compiler revision dependent problem in such a way that the code does not need to be conditionalized on the compiler revision to avoid being broken? -- Terry To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-current" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?3E59B5DA.66EDBC85>