All of lore.kernel.org
 help / color / mirror / Atom feed
* container_of
@ 2008-12-19 17:56 Julia Lawall
  2008-12-20 11:54 ` container_of Darren Jenkins
                   ` (5 more replies)
  0 siblings, 6 replies; 13+ messages in thread
From: Julia Lawall @ 2008-12-19 17:56 UTC (permalink / raw)
  To: kernel-janitors

Considering code such as the following (drivers/acpi/osl.c):

 	struct acpi_os_dpc *dpc = container_of(work, struct acpi_os_dpc, work);
	if (!dpc) {
 		printk(KERN_ERR PREFIX "Invalid (NULL) context\n");
 		return;
 	}

Is it very likely that container_of can return NULL?  Container_of 
computes an offset from a pointer, so I have the impression that if given 
a NULL value it would normally return a negative number (or a very large 
positive one, depending on how it is interpreted).

thanks,
julia

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

* Re: container_of
  2008-12-19 17:56 container_of Julia Lawall
@ 2008-12-20 11:54 ` Darren Jenkins
  2008-12-20 18:19 ` container_of Julia Lawall
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: Darren Jenkins @ 2008-12-20 11:54 UTC (permalink / raw)
  To: kernel-janitors

Hello Julia,

I think pointer signedness is architecture dependant, but I have never
seen an architecture crazy enough to use signed memory addressing.

I haven't looked at the associated code (because it is late here and I
am tired),  but it looks like this bit is just checking if the struct
is NULL, not the struct member.
Depending on the associated code, this is probably the sensible thing
to do, as the memory will be allocated for the struct, so if the
allocation fails the pointer to the base of the struct will be NULL
and the pointer to the struct member will be "NULL + offsetof(struct,
member)" ie probably not null.



On Sat, Dec 20, 2008 at 4:56 AM, Julia Lawall <julia@diku.dk> wrote:
> Considering code such as the following (drivers/acpi/osl.c):
>
>        struct acpi_os_dpc *dpc = container_of(work, struct acpi_os_dpc, work);
>        if (!dpc) {
>                printk(KERN_ERR PREFIX "Invalid (NULL) context\n");
>                return;
>        }
>
> Is it very likely that container_of can return NULL?  Container_of
> computes an offset from a pointer, so I have the impression that if given
> a NULL value it would normally return a negative number (or a very large
> positive one, depending on how it is interpreted).
>
> thanks,
> julia
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

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

* Re: container_of
  2008-12-19 17:56 container_of Julia Lawall
  2008-12-20 11:54 ` container_of Darren Jenkins
@ 2008-12-20 18:19 ` Julia Lawall
  2008-12-23  5:37 ` container_of Greg KH
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: Julia Lawall @ 2008-12-20 18:19 UTC (permalink / raw)
  To: kernel-janitors

On Sat, 20 Dec 2008, Darren Jenkins wrote:

> Hello Julia,
> 
> I think pointer signedness is architecture dependant, but I have never
> seen an architecture crazy enough to use signed memory addressing.
> 
> I haven't looked at the associated code (because it is late here and I
> am tired),  but it looks like this bit is just checking if the struct
> is NULL, not the struct member.
> Depending on the associated code, this is probably the sensible thing
> to do, as the memory will be allocated for the struct, so if the
> allocation fails the pointer to the base of the struct will be NULL
> and the pointer to the struct member will be "NULL + offsetof(struct,
> member)" ie probably not null.

OK, I see, good point.

thanks,
julia

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

* Re: container_of
  2008-12-19 17:56 container_of Julia Lawall
  2008-12-20 11:54 ` container_of Darren Jenkins
  2008-12-20 18:19 ` container_of Julia Lawall
@ 2008-12-23  5:37 ` Greg KH
  2008-12-23  6:01 ` container_of Julia Lawall
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: Greg KH @ 2008-12-23  5:37 UTC (permalink / raw)
  To: kernel-janitors

On Fri, Dec 19, 2008 at 06:56:46PM +0100, Julia Lawall wrote:
> Considering code such as the following (drivers/acpi/osl.c):
> 
>  	struct acpi_os_dpc *dpc = container_of(work, struct acpi_os_dpc, work);
> 	if (!dpc) {
>  		printk(KERN_ERR PREFIX "Invalid (NULL) context\n");
>  		return;
>  	}
> 
> Is it very likely that container_of can return NULL?  Container_of 
> computes an offset from a pointer, so I have the impression that if given 
> a NULL value it would normally return a negative number (or a very large 
> positive one, depending on how it is interpreted).

You are correct, that check will never happen, and should be removed.

thanks,

greg k-h

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

* Re: container_of
  2008-12-19 17:56 container_of Julia Lawall
                   ` (2 preceding siblings ...)
  2008-12-23  5:37 ` container_of Greg KH
@ 2008-12-23  6:01 ` Julia Lawall
  2008-12-23  6:09 ` container_of Greg KH
  2008-12-23  7:00 ` container_of Julia Lawall
  5 siblings, 0 replies; 13+ messages in thread
From: Julia Lawall @ 2008-12-23  6:01 UTC (permalink / raw)
  To: kernel-janitors

On Mon, 22 Dec 2008, Greg KH wrote:

> On Fri, Dec 19, 2008 at 06:56:46PM +0100, Julia Lawall wrote:
> > Considering code such as the following (drivers/acpi/osl.c):
> > 
> >  	struct acpi_os_dpc *dpc = container_of(work, struct acpi_os_dpc, work);
> > 	if (!dpc) {
> >  		printk(KERN_ERR PREFIX "Invalid (NULL) context\n");
> >  		return;
> >  	}
> > 
> > Is it very likely that container_of can return NULL?  Container_of 
> > computes an offset from a pointer, so I have the impression that if given 
> > a NULL value it would normally return a negative number (or a very large 
> > positive one, depending on how it is interpreted).
> 
> You are correct, that check will never happen, and should be removed.

Actually, someone else pointed out that one could call the function with 
&x->fld, and if x is NULL, then container_of(&x->fld,...) would go back 
down to NULL.  I haven't looked at the call sites though to see if that 
can happen.

julia

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

* Re: container_of
  2008-12-19 17:56 container_of Julia Lawall
                   ` (3 preceding siblings ...)
  2008-12-23  6:01 ` container_of Julia Lawall
@ 2008-12-23  6:09 ` Greg KH
  2008-12-23  7:00 ` container_of Julia Lawall
  5 siblings, 0 replies; 13+ messages in thread
From: Greg KH @ 2008-12-23  6:09 UTC (permalink / raw)
  To: kernel-janitors

On Tue, Dec 23, 2008 at 07:01:59AM +0100, Julia Lawall wrote:
> On Mon, 22 Dec 2008, Greg KH wrote:
> 
> > On Fri, Dec 19, 2008 at 06:56:46PM +0100, Julia Lawall wrote:
> > > Considering code such as the following (drivers/acpi/osl.c):
> > > 
> > >  	struct acpi_os_dpc *dpc = container_of(work, struct acpi_os_dpc, work);
> > > 	if (!dpc) {
> > >  		printk(KERN_ERR PREFIX "Invalid (NULL) context\n");
> > >  		return;
> > >  	}
> > > 
> > > Is it very likely that container_of can return NULL?  Container_of 
> > > computes an offset from a pointer, so I have the impression that if given 
> > > a NULL value it would normally return a negative number (or a very large 
> > > positive one, depending on how it is interpreted).
> > 
> > You are correct, that check will never happen, and should be removed.
> 
> Actually, someone else pointed out that one could call the function with 
> &x->fld, and if x is NULL, then container_of(&x->fld,...) would go back 
> down to NULL.  I haven't looked at the call sites though to see if that 
> can happen.

No, if you were to evaluate x->fld, you would oops :)

So again, the check is pointless.  And I say this as someone who has
written such checks many times in the past...

thanks,

greg k-h

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

* Re: container_of
  2008-12-19 17:56 container_of Julia Lawall
                   ` (4 preceding siblings ...)
  2008-12-23  6:09 ` container_of Greg KH
@ 2008-12-23  7:00 ` Julia Lawall
  5 siblings, 0 replies; 13+ messages in thread
From: Julia Lawall @ 2008-12-23  7:00 UTC (permalink / raw)
  To: kernel-janitors

On Mon, 22 Dec 2008, Greg KH wrote:

> On Tue, Dec 23, 2008 at 07:01:59AM +0100, Julia Lawall wrote:
> > On Mon, 22 Dec 2008, Greg KH wrote:
> > 
> > > On Fri, Dec 19, 2008 at 06:56:46PM +0100, Julia Lawall wrote:
> > > > Considering code such as the following (drivers/acpi/osl.c):
> > > > 
> > > >  	struct acpi_os_dpc *dpc = container_of(work, struct acpi_os_dpc, work);
> > > > 	if (!dpc) {
> > > >  		printk(KERN_ERR PREFIX "Invalid (NULL) context\n");
> > > >  		return;
> > > >  	}
> > > > 
> > > > Is it very likely that container_of can return NULL?  Container_of 
> > > > computes an offset from a pointer, so I have the impression that if given 
> > > > a NULL value it would normally return a negative number (or a very large 
> > > > positive one, depending on how it is interpreted).
> > > 
> > > You are correct, that check will never happen, and should be removed.
> > 
> > Actually, someone else pointed out that one could call the function with 
> > &x->fld, and if x is NULL, then container_of(&x->fld,...) would go back 
> > down to NULL.  I haven't looked at the call sites though to see if that 
> > can happen.
> 
> No, if you were to evaluate x->fld, you would oops :)

&x->fld doesn't oops, though.  It just adds the offset of the field fld to 
x.

julia

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

* container_of
@ 2015-01-17 16:32 Simon Brand
  2015-01-17 16:58 ` container_of Manish Katiyar
  2015-01-17 19:13 ` container_of Anish Kumar
  0 siblings, 2 replies; 13+ messages in thread
From: Simon Brand @ 2015-01-17 16:32 UTC (permalink / raw)
  To: kernelnewbies

Good evening,

i read the article about the container_of function:
http://www.kroah.com/log/linux/container_of.html

 I understand what it does and how it works.

I thought, it could be simplified by replacing it through this:

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

Original:
#define container_of(ptr, type, member) ({ \
                const typeof( ((type *)0)->member ) *__mptr = (ptr); 
                (type *)( (char *)__mptr - offsetof(type,member) );})


ptr has the type of a pointer to the member, which should be the same
as __mptr? The value should although be the same.

First I tried it in a self written script, then replaced it in
include/linux/kernel.h and compiled it as usermode linux -> working
well.

Then I compiled it and run it in a VM, but it is not working.

Can you please explain to me, why the original version is always working
and "mine" is not? 

Thank you for your time!

Regards,
Simon

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

* container_of
  2015-01-17 16:32 container_of Simon Brand
@ 2015-01-17 16:58 ` Manish Katiyar
  2015-01-17 19:45   ` container_of Simon Brand
  2015-01-17 19:13 ` container_of Anish Kumar
  1 sibling, 1 reply; 13+ messages in thread
From: Manish Katiyar @ 2015-01-17 16:58 UTC (permalink / raw)
  To: kernelnewbies

on Sat, Jan 17, 2015 at 8:32 AM, Simon Brand <simon.brand@postadigitale.de>
wrote:

> Good evening,
>
> i read the article about the container_of function:
> http://www.kroah.com/log/linux/container_of.html
>
>  I understand what it does and how it works.
>
> I thought, it could be simplified by replacing it through this:
>
> #define container_of(ptr, type, member) ({ \
>                 (type *)( (char *)ptr - offsetof(type,member) );})
>
> Original:
> #define container_of(ptr, type, member) ({ \
>                 const typeof( ((type *)0)->member ) *__mptr = (ptr);
>                 (type *)( (char *)__mptr - offsetof(type,member) );})
>
>
> ptr has the type of a pointer to the member, which should be the same
> as __mptr? The value should although be the same.
>
> First I tried it in a self written script, then replaced it in
> include/linux/kernel.h and compiled it as usermode linux -> working
> well.
>
> Then I compiled it and run it in a VM, but it is not working.
>
> Can you please explain to me, why the original version is always working
> and "mine" is not?
>


Have you searched through archives. Exactly 7 years ago, I had the same
question.

http://comments.gmane.org/gmane.linux.kernel.kernelnewbies/24141

Thanks -
Manish




> _______________________________________________
> Kernelnewbies mailing list
> Kernelnewbies at kernelnewbies.org
> http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20150117/d27a94a6/attachment.html 

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

* container_of
  2015-01-17 16:32 container_of Simon Brand
  2015-01-17 16:58 ` container_of Manish Katiyar
@ 2015-01-17 19:13 ` Anish Kumar
  1 sibling, 0 replies; 13+ messages in thread
From: Anish Kumar @ 2015-01-17 19:13 UTC (permalink / raw)
  To: kernelnewbies





> On Jan 17, 2015, at 8:32 AM, Simon Brand <simon.brand@postadigitale.de> wrote:
> 
> Good evening,
> 
> i read the article about the container_of function:
> http://www.kroah.com/log/linux/container_of.html
> 
> I understand what it does and how it works.
> 
> I thought, it could be simplified by replacing it through this:
> 
> #define container_of(ptr, type, member) ({ \
>                (type *)( (char *)ptr - offsetof(type,member) );})
> 
> Original:
> #define container_of(ptr, type, member) ({ \
>                const typeof( ((type *)0)->member ) *__mptr = (ptr); 
>                (type *)( (char *)__mptr - offsetof(type,member) );})
> 
> 
> ptr has the type of a pointer to the member, which should be the same
> as __mptr? The value should although be the same.
> 
> First I tried it in a self written script, then replaced it in
> include/linux/kernel.h and compiled it as usermode linux -> working
> well.
> 
> Then I compiled it and run it in a VM, but it is not working.
What do you mean by that? What is not working?
> 
> Can you please explain to me, why the original version is always working
> and "mine" is not? 
> 
> Thank you for your time!
> 
> Regards,
> Simon
> 
> _______________________________________________
> Kernelnewbies mailing list
> Kernelnewbies at kernelnewbies.org
> http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

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

* container_of
  2015-01-17 16:58 ` container_of Manish Katiyar
@ 2015-01-17 19:45   ` Simon Brand
  2015-01-17 19:53     ` container_of Mike Krinkin
  0 siblings, 1 reply; 13+ messages in thread
From: Simon Brand @ 2015-01-17 19:45 UTC (permalink / raw)
  To: kernelnewbies

Am Sat, 17 Jan 2015 08:58:13 -0800
schrieb Manish Katiyar <mkatiyar@gmail.com>:

> Have you searched through archives. Exactly 7 years ago, I had the
> same question.
> 
> http://comments.gmane.org/gmane.linux.kernel.kernelnewbies/24141
> 

No, sorry, I missed that :-x
Thank you.

To get this straight: it is only to produce a warning at compile time,
when it is misused?


I compiled the kernel two times, one time with the original code and
one time with
#define container_of(ptr, type, member) ({			\
	(type *)( (char *)ptr - offsetof(type,member) );})


The secound kernel does not work proberly. 
First there is a kernel BUG at include/drm/drm_mm.h:145 at every boot:
http://sprunge.us/MdDa

Secound the kernel hangs on reboot and poweroff:
reboot:
http://picpaste.de/pics/8a041c11f3f5e24faebc1abb41b1db3f.1421523345.png
poweroff:
http://picpaste.de/pics/b1ab5225f37572a31b43e2fb8526e890.1421523472.png

Third for example startx only produces the output:
waiting for X server to begin accepting connections

There is no further output in dmesg.

The X server starts correctly with the first/original kernel.

I compiled both kernels with following config:
https://projects.archlinux.org/svntogit/packages.git/plain/trunk/config.x86_64?h=packages/linux
and following patches:
https://projects.archlinux.org/svntogit/packages.git/plain/trunk/0001-drm-i915-Disallow-pin-ioctl-completely-for-kms-drive.patch?h=packages/linux
https://projects.archlinux.org/svntogit/packages.git/plain/trunk/change-default-console-loglevel.patch?h=packages/linux


I compiled a little c code with both defines and gcc is producing
another binary, but both are working as they should.


Thank you for your reply.
Simon

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

* container_of
  2015-01-17 19:45   ` container_of Simon Brand
@ 2015-01-17 19:53     ` Mike Krinkin
  2015-01-19 15:18       ` container_of Simon Brand
  0 siblings, 1 reply; 13+ messages in thread
From: Mike Krinkin @ 2015-01-17 19:53 UTC (permalink / raw)
  To: kernelnewbies

Hi, Simon

> I compiled the kernel two times, one time with the original code and
> one time with
> #define container_of(ptr, type, member) ({			\
> 	(type *)( (char *)ptr - offsetof(type,member) );})

try with following version:

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

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

* container_of
  2015-01-17 19:53     ` container_of Mike Krinkin
@ 2015-01-19 15:18       ` Simon Brand
  0 siblings, 0 replies; 13+ messages in thread
From: Simon Brand @ 2015-01-19 15:18 UTC (permalink / raw)
  To: kernelnewbies

Am Sat, 17 Jan 2015 22:53:10 +0300
schrieb Mike Krinkin <krinkin.m.u@gmail.com>:

> 
> #define container_of(ptr, type, member) ({ \
> 	(type *)((char *)(ptr) - offsetof(type, member));})
> 

Thank you Mike, this is working!

ptr is not evaluated before the text is replaced -.-
I should know this...

Thank you very much!

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

end of thread, other threads:[~2015-01-19 15:18 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-01-17 16:32 container_of Simon Brand
2015-01-17 16:58 ` container_of Manish Katiyar
2015-01-17 19:45   ` container_of Simon Brand
2015-01-17 19:53     ` container_of Mike Krinkin
2015-01-19 15:18       ` container_of Simon Brand
2015-01-17 19:13 ` container_of Anish Kumar
  -- strict thread matches above, loose matches on Subject: below --
2008-12-19 17:56 container_of Julia Lawall
2008-12-20 11:54 ` container_of Darren Jenkins
2008-12-20 18:19 ` container_of Julia Lawall
2008-12-23  5:37 ` container_of Greg KH
2008-12-23  6:01 ` container_of Julia Lawall
2008-12-23  6:09 ` container_of Greg KH
2008-12-23  7:00 ` container_of Julia Lawall

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.