Re: CVS commit: src/usr.bin/quota
On Mon, Jan 30, 2012 at 06:14:43AM +, David A. Holland wrote: Module Name: src Committed By: dholland Date: Mon Jan 30 06:14:43 UTC 2012 Modified Files: src/usr.bin/quota: quotautil.c quotautil.h Log Message: Remove stray p in identifier name. This has (as far as I can tell) prevented quotacheck and other old-style quota bits from working since last March. just to make it clear: this bugs only shows up if you use userquota/groupquota in fstab without an explicit quota file name; that's probably why it was not noticed. -- Manuel Bouyer bou...@antioche.eu.org NetBSD: 26 ans d'experience feront toujours la difference --
Re: CVS commit: src/sys/kern
On Tue, Jan 31, 2012 at 07:11:38PM +, Alexander Nasonov wrote: Module Name: src Committed By: alnsn Date: Tue Jan 31 19:11:38 UTC 2012 Modified Files: src/sys/kern: subr_pcq.c Log Message: Replace offsetof(pcq_t, pcq_items[nitems]) with sizeof(pcq_t) + sizeof(void *[nitems]). Yuck. The offsetof() way was much more readable. Please put it back the old way. If you have to, provide and use a runtime_offsetof() that DTRT. Dave -- David Young dyo...@pobox.comUrbana, IL(217) 721-9981
Re: CVS commit: src/sys/kern
David Young wrote: Yuck. The offsetof() way was much more readable. Yes, it's more readable but it's not standard C99. And it's confusing to use offsetof when you want to use sizeof. Please put it back the old way. If you have to, provide and use a runtime_offsetof() that DTRT. DTRT TWW ;-) What about something like this (untested)? /* * Return a size of a structure s with flexible-array member m * with n elements. */ #define sizeof_fam(s, m, n) (sizeof(s) + sizeof(((s *)NULL)-m[0]) * (n)) and use like this: sizeof_fam(struct pcq, pcq_items, nitems); Alex
Re: CVS commit: src/sys/kern
On Tue, Jan 31, 2012 at 07:32:52PM +, Alexander Nasonov wrote: What about something like this (untested)? /* * Return a size of a structure s with flexible-array member m * with n elements. */ #define sizeof_fam(s, m, n) (sizeof(s) + sizeof(((s *)NULL)-m[0]) * (n)) That's still not necessarily optimal, depending on the padding rules of the platform. You want to do offsetof(s, m[0]) + n * sizeof((s*)NULL-m[0]). Joerg
Re: CVS commit: src/sys/kern
Joerg Sonnenberger wrote: On Tue, Jan 31, 2012 at 07:32:52PM +, Alexander Nasonov wrote: #define sizeof_fam(s, m, n) (sizeof(s) + sizeof(((s *)NULL)-m[0]) * (n)) That's still not necessarily optimal, depending on the padding rules of the platform. You want to do offsetof(s, m[0]) + n * sizeof((s*)NULL-m[0]). I'm aware of this but I was merely following examples from the standard. More specifically I was following DR 282 (flexible array members struct padding) which was incorporated into TC2. http://std.dkuug.dk/jtc1/sc22/wg14/www/docs/dr_282.htm I can add a comment about padding to sizeof_fam. If there is no padding, my and your expression should return same values. Alex
Re: CVS commit: src/sys/kern
On Tue, Jan 31, 2012 at 07:50:26PM +, Alexander Nasonov wrote: Joerg Sonnenberger wrote: On Tue, Jan 31, 2012 at 07:32:52PM +, Alexander Nasonov wrote: #define sizeof_fam(s, m, n) (sizeof(s) + sizeof(((s *)NULL)-m[0]) * (n)) That's still not necessarily optimal, depending on the padding rules of the platform. You want to do offsetof(s, m[0]) + n * sizeof((s*)NULL-m[0]). I'm aware of this but I was merely following examples from the standard. More specifically I was following DR 282 (flexible array members struct padding) which was incorporated into TC2. http://std.dkuug.dk/jtc1/sc22/wg14/www/docs/dr_282.htm I can add a comment about padding to sizeof_fam. If there is no padding, my and your expression should return same values. And if there is padding, and it is after the last field (and a few other clauses) your scheme will allocate a longer buffer than needed. The advantage of Joerg's is that you don't have to 'know' the type of the member. Sudden barin explosion - how about (untested): #define sizeof_var_struct(s, m, c) \ offsetof(s, m[0]) + (c) * (offsetof(s, m[1]) - offsetof(s, m[0])) David -- David Laight: da...@l8s.co.uk
Re: CVS commit: src/sys/kern
Joerg Sonnenberger wrote: That's still not necessarily optimal, depending on the padding rules of the platform. You want to do offsetof(s, m[0]) + n * sizeof((s*)NULL-m[0]). Using m[0] inside offsetof is non-standard but I think this will work: offsetof(s, m) + n * sizeof((s*)NULL-m[0]). Alex
Re: CVS commit: src/sys/kern
On Tue, Jan 31, 2012 at 09:51:17PM +, Alexander Nasonov wrote: Joerg Sonnenberger wrote: That's still not necessarily optimal, depending on the padding rules of the platform. You want to do offsetof(s, m[0]) + n * sizeof((s*)NULL-m[0]). Using m[0] inside offsetof is non-standard but I think this will work: offsetof(s, m) + n * sizeof((s*)NULL-m[0]). I don't believe there is a problem using m[0], just m[non-constant-expr]. OTOH 'sizeof (s *)0-m[0]' might be deemed invalid (because it has an inferred dereference of NULL. David -- David Laight: da...@l8s.co.uk
Re: CVS commit: src/sys/kern
David Laight wrote: I don't believe there is a problem using m[0], just m[non-constant-expr]. Practically speaking, m[0] is not a problem but the standard says ... to the structure member (designated by member-designator), ... ^^ OTOH 'sizeof (s *)0-m[0]' might be deemed invalid (because it has an inferred dereference of NULL. Oh yes. I can change it to: extern void *foo(void); and then sizeof(((s *)foo())-m[0]) but NULL should work too. So, if we reached a consensus on this, where should I add the macro? Alex
Re: CVS commit: src/sys/kern
On Jan 31, 2012, at 3:53 PM, Alexander Nasonov wrote: David Laight wrote: I don't believe there is a problem using m[0], just m[non-constant-expr]. Practically speaking, m[0] is not a problem but the standard says ... to the structure member (designated by member-designator), ... ^^ OTOH 'sizeof (s *)0-m[0]' might be deemed invalid (because it has an inferred dereference of NULL. Oh yes. I can change it to: extern void *foo(void); and then sizeof(((s *)foo())-m[0]) but NULL should work too. So, if we reached a consensus on this, where should I add the macro? There's no NULL dereference, so it is fine (completely standards conforming). The sizeof operator is a compile time construct that evaluates its argument for its type only, so no dereference is happening, so any value for the pointer is OK. In the absence of a local variable, 0 is just as valid as anything else. Warner
Re: CVS commit: src/sys/kern
On Tue, Jan 31, 2012 at 22:53:37 +, Alexander Nasonov wrote: David Laight wrote: I don't believe there is a problem using m[0], just m[non-constant-expr]. Practically speaking, m[0] is not a problem but the standard says ... to the structure member (designated by member-designator), ... ^^ Well, the standard also says that: The type and member designator shall be such that given static type t; then the expression (t.member-designator) evaluates to an address constant. (If the specified member is a bit-field, the behavior is undefined.) Note that the standard doesn't actually bother to define memeber-designator formally, but see 6.7.8 Initialization. We can haggle about the finer points of specific wording used in the relevant paragraphs, but I think the intended meaning is pretty clear. -uwe
