container_of

container_of

[Julia Lawall]

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

Re: container_of

[Darren Jenkins]

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
>

Re: container_of

[Julia Lawall]

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

Re: container_of

[Greg KH]

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

Re: container_of

[Julia Lawall]

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

Re: container_of

[Greg KH]

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

Re: container_of

[Julia Lawall]

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

container_of

[Simon Brand]

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

container_of

[Manish Katiyar]

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 

container_of

[Simon Brand]

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

container_of

[Mike Krinkin]

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));})

container_of

[Simon Brand]

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!

container_of

[Anish Kumar]

> 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