Util-Linux Archive on lore.kernel.org
 help / Atom feed
* [PATCH] include/c: re-add type checking in container_of()
@ 2018-12-02 18:23 Ruediger Meier
  2018-12-03 10:23 ` Karel Zak
  0 siblings, 1 reply; 4+ messages in thread
From: Ruediger Meier @ 2018-12-02 18:23 UTC (permalink / raw)
  To: util-linux

From: Ruediger Meier <ruediger.meier@ga-group.nl>

This reverts parts of commit eb06d5d4, which seems to be based on
Linux kernel commit c7acec71. Unlike the original kernel patch we did
not add that even stronger type checking by using macro BUILD_BUG_ON_MSG.
So basically we removed a useful warning when compiling such
broken code:

      struct st {
            int a;
            char b;
      };
      struct st t = { .a = 1, .b = 2 };
      struct st *x = container_of(&t.a, struct st, b);
      printf("%p %p\n", (void *)&t, (void *)x);

Moreover we also introduced a new compiler warning for intel/icc:
   "arithmetic on pointer to void or function type"

Let's just revert the update of container_of() because adding a
kernel-like BUILD_BUG_ON_MSG would be too much noise and also
problematic (see kernel commit c03567a8). Also note that the original
problem addressed by the kernel commit seems to be only reproducible
with gcc 4.9, not with any later gcc nor clang,icc. Moreover, currently
we have no such use-case in the UL sources anyways.

Signed-off-by: Ruediger Meier <ruediger.meier@ga-group.nl>
---
 include/c.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/c.h b/include/c.h
index 8b2a2d19a..f1e329819 100644
--- a/include/c.h
+++ b/include/c.h
@@ -147,8 +147,8 @@
  */
 #ifndef container_of
 #define container_of(ptr, type, member) __extension__ ({	\
-	void *__mptr = (void *)(ptr);				\
-	((type *)(__mptr - offsetof(type, member))); })
+	const __typeof__( ((type *)0)->member ) *__mptr = (ptr); \
+	(type *)( (char *)__mptr - offsetof(type,member) );})
 #endif
 
 #ifndef HAVE_PROGRAM_INVOCATION_SHORT_NAME
-- 
2.13.7


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] include/c: re-add type checking in container_of()
  2018-12-02 18:23 [PATCH] include/c: re-add type checking in container_of() Ruediger Meier
@ 2018-12-03 10:23 ` Karel Zak
  2018-12-03 18:36   ` Ruediger Meier
  2018-12-03 21:37   ` Ruediger Meier
  0 siblings, 2 replies; 4+ messages in thread
From: Karel Zak @ 2018-12-03 10:23 UTC (permalink / raw)
  To: Ruediger Meier; +Cc: util-linux

On Sun, Dec 02, 2018 at 07:23:45PM +0100, Ruediger Meier wrote:
> From: Ruediger Meier <ruediger.meier@ga-group.nl>
> 
> This reverts parts of commit eb06d5d4, which seems to be based on
> Linux kernel commit c7acec71. Unlike the original kernel patch we did
> not add that even stronger type checking by using macro BUILD_BUG_ON_MSG.
> So basically we removed a useful warning when compiling such
> broken code:
> 
>       struct st {
>             int a;
>             char b;
>       };
>       struct st t = { .a = 1, .b = 2 };
>       struct st *x = container_of(&t.a, struct st, b);
>       printf("%p %p\n", (void *)&t, (void *)x);
> 
> Moreover we also introduced a new compiler warning for intel/icc:
>    "arithmetic on pointer to void or function type"
> 
> Let's just revert the update of container_of() because adding a
> kernel-like BUILD_BUG_ON_MSG would be too much noise and also
> problematic (see kernel commit c03567a8). Also note that the original
> problem addressed by the kernel commit seems to be only reproducible
> with gcc 4.9, not with any later gcc nor clang,icc. Moreover, currently
> we have no such use-case in the UL sources anyways.

Interesting, I do not see any issue with list_entry(() now ;-)

Applied, thanks!

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] include/c: re-add type checking in container_of()
  2018-12-03 10:23 ` Karel Zak
@ 2018-12-03 18:36   ` Ruediger Meier
  2018-12-03 21:37   ` Ruediger Meier
  1 sibling, 0 replies; 4+ messages in thread
From: Ruediger Meier @ 2018-12-03 18:36 UTC (permalink / raw)
  To: Karel Zak; +Cc: util-linux

On Monday 03 December 2018, Karel Zak wrote:
> On Sun, Dec 02, 2018 at 07:23:45PM +0100, Ruediger Meier wrote:
> > From: Ruediger Meier <ruediger.meier@ga-group.nl>
> >
> > This reverts parts of commit eb06d5d4, which seems to be based on
> > Linux kernel commit c7acec71. Unlike the original kernel patch we
> > did not add that even stronger type checking by using macro
> > BUILD_BUG_ON_MSG. So basically we removed a useful warning when
> > compiling such broken code:
> >
> >       struct st {
> >             int a;
> >             char b;
> >       };
> >       struct st t = { .a = 1, .b = 2 };
> >       struct st *x = container_of(&t.a, struct st, b);
> >       printf("%p %p\n", (void *)&t, (void *)x);
> >
> > Moreover we also introduced a new compiler warning for intel/icc:
> >    "arithmetic on pointer to void or function type"
> >
> > Let's just revert the update of container_of() because adding a
> > kernel-like BUILD_BUG_ON_MSG would be too much noise and also
> > problematic (see kernel commit c03567a8). Also note that the
> > original problem addressed by the kernel commit seems to be only
> > reproducible with gcc 4.9, not with any later gcc nor clang,icc.
> > Moreover, currently we have no such use-case in the UL sources
> > anyways.
>
> Interesting, I do not see any issue with list_entry(() now ;-)

Well, actually the macro would perfectly work as simple as this:

    #define container_of(ptr, type, member) ( \
       (type *)((char *)ptr - offsetof(type,member)) )


All additional stuff is only to get some warnings if programmers are 
using the macro wrongly. Unfortunately also some false positive 
warnings. That was and is still the only problem here ... 
overengineering ;)

cu,
Rudi

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] include/c: re-add type checking in container_of()
  2018-12-03 10:23 ` Karel Zak
  2018-12-03 18:36   ` Ruediger Meier
@ 2018-12-03 21:37   ` Ruediger Meier
  1 sibling, 0 replies; 4+ messages in thread
From: Ruediger Meier @ 2018-12-03 21:37 UTC (permalink / raw)
  To: Karel Zak; +Cc: util-linux

On Monday 03 December 2018, Karel Zak wrote:
> On Sun, Dec 02, 2018 at 07:23:45PM +0100, Ruediger Meier wrote:
> > From: Ruediger Meier <ruediger.meier@ga-group.nl>
> >
> > This reverts parts of commit eb06d5d4, which seems to be based on
> > Linux kernel commit c7acec71. Unlike the original kernel patch we
> > did not add that even stronger type checking by using macro
> > BUILD_BUG_ON_MSG. So basically we removed a useful warning when
> > compiling such broken code:
> >
> >       struct st {
> >             int a;
> >             char b;
> >       };
> >       struct st t = { .a = 1, .b = 2 };
> >       struct st *x = container_of(&t.a, struct st, b);
> >       printf("%p %p\n", (void *)&t, (void *)x);
> >
> > Moreover we also introduced a new compiler warning for intel/icc:
> >    "arithmetic on pointer to void or function type"
> >
> > Let's just revert the update of container_of() because adding a
> > kernel-like BUILD_BUG_ON_MSG would be too much noise and also
> > problematic (see kernel commit c03567a8). Also note that the
> > original problem addressed by the kernel commit seems to be only
> > reproducible with gcc 4.9, not with any later gcc nor clang,icc.
> > Moreover, currently we have no such use-case in the UL sources
> > anyways.
>
> Interesting, I do not see any issue with list_entry(() now ;-)


BTW this is a nice webtool where you can see the issues quickly:

https://godbolt.org/z/Z2XvPc

cu,
Rudi

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-02 18:23 [PATCH] include/c: re-add type checking in container_of() Ruediger Meier
2018-12-03 10:23 ` Karel Zak
2018-12-03 18:36   ` Ruediger Meier
2018-12-03 21:37   ` Ruediger Meier

Util-Linux Archive on lore.kernel.org

Archives are clonable: git clone --mirror https://lore.kernel.org/util-linux/0 util-linux/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 util-linux util-linux/ https://lore.kernel.org/util-linux \
		util-linux@vger.kernel.org util-linux@archiver.kernel.org
	public-inbox-index util-linux


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.util-linux


AGPL code for this site: git clone https://public-inbox.org/ public-inbox