Re: svn commit: samba r11705 - in branches/SAMBA_4_0/source/libnet: .

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: samba r11705 - in branches/SAMBA_4_0/source/libnet: .

Stefan Metzmacher-2
On Sat, Nov 12, 2005 at 09:44:42PM +0000, [hidden email] wrote:

> Fix segfaulting create user function.
> Changeset:
> Modified: branches/SAMBA_4_0/source/libnet/libnet_user.c
> ===================================================================
> --- branches/SAMBA_4_0/source/libnet/libnet_user.c 2005-11-12 18:22:12 UTC (rev 11704)
> +++ branches/SAMBA_4_0/source/libnet/libnet_user.c 2005-11-12 21:44:42 UTC (rev 11705)
> @@ -31,10 +31,14 @@
>   struct libnet_Lookup fp;
>   struct libnet_rpc_domain_open dom_io;
>   struct libnet_rpc_useradd user_io;
> + const char *address;
>  
> + address = talloc_array(mem_ctx, const char, 8);
> +
>   /* find domain pdc */
>   fp.in.hostname    = r->in.domain_name;
>   fp.in.methods     = NULL;
> + fp.out.address    = &address;
>  
>   status = libnet_LookupPdc(ctx, mem_ctx, &fp);
>   if (!NT_STATUS_IS_OK(status)) return status;

Hi Rafal,

can you explain that a bit more?

why an array of 8 characters? and why the caller need to set the out parameter?

metze
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: samba r11705 - in branches/SAMBA_4_0/source/libnet: .

Rafal Szczesniak
On Sun, Nov 13, 2005 at 02:12:56PM +0100, Stefan Metzmacher wrote:

> On Sat, Nov 12, 2005 at 09:44:42PM +0000, [hidden email] wrote:
> >   struct libnet_rpc_domain_open dom_io;
> >   struct libnet_rpc_useradd user_io;
> > + const char *address;
> >  
> > + address = talloc_array(mem_ctx, const char, 8);
> > +
> >   /* find domain pdc */
> >   fp.in.hostname    = r->in.domain_name;
> >   fp.in.methods     = NULL;
> > + fp.out.address    = &address;
> >  
> >   status = libnet_LookupPdc(ctx, mem_ctx, &fp);
> >   if (!NT_STATUS_IS_OK(status)) return status;
>
> Hi Rafal,
>
> can you explain that a bit more?
>
> why an array of 8 characters? and why the caller need to set the out parameter?
I'm sorry, this is mistake. It should be long enough to store ip
address. Thanks for catching that.
Address argument is just like [ref] - you have to pass a
pointer to allocated buffer for function to fill in the result.
Otherwise resolve_name function segfaults.


cheers,
--
Rafal Szczesniak
Samba Team member  http://www.samba.org


signature.asc (196 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: samba r11705 - in branches/SAMBA_4_0/source/libnet: .

Andrew Bartlett
On Sun, 2005-11-13 at 19:38 +0100, Rafal Szczesniak wrote:

> On Sun, Nov 13, 2005 at 02:12:56PM +0100, Stefan Metzmacher wrote:
> > On Sat, Nov 12, 2005 at 09:44:42PM +0000, [hidden email] wrote:
> > >   struct libnet_rpc_domain_open dom_io;
> > >   struct libnet_rpc_useradd user_io;
> > > + const char *address;
> > >  
> > > + address = talloc_array(mem_ctx, const char, 8);
> > > +
> > >   /* find domain pdc */
> > >   fp.in.hostname    = r->in.domain_name;
> > >   fp.in.methods     = NULL;
> > > + fp.out.address    = &address;
> > >  
> > >   status = libnet_LookupPdc(ctx, mem_ctx, &fp);
> > >   if (!NT_STATUS_IS_OK(status)) return status;
> >
> > Hi Rafal,
> >
> > can you explain that a bit more?
> >
> > why an array of 8 characters? and why the caller need to set the out parameter?
>
> I'm sorry, this is mistake. It should be long enough to store ip
> address. Thanks for catching that.
> Address argument is just like [ref] - you have to pass a
> pointer to allocated buffer for function to fill in the result.
> Otherwise resolve_name function segfaults.
I really don't think we want those semantics in libnet.  Why can't
libnet_LookupPdc do the allocation?

Andrew Bartlett

--
Andrew Bartlett                                http://samba.org/~abartlet/
Authentication Developer, Samba Team           http://samba.org
Student Network Administrator, Hawker College  http://hawkerc.net

signature.asc (196 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: samba r11705 - in branches/SAMBA_4_0/source/libnet: .

Rafal Szczesniak
On Mon, Nov 14, 2005 at 08:39:51AM +1100, Andrew Bartlett wrote:

> On Sun, 2005-11-13 at 22:36 +0100, Rafal Szczesniak wrote:
> > On Mon, Nov 14, 2005 at 08:05:07AM +1100, Andrew Bartlett wrote:
> > > On Sun, 2005-11-13 at 19:38 +0100, Rafal Szczesniak wrote:
> > > > On Sun, Nov 13, 2005 at 02:12:56PM +0100, Stefan Metzmacher wrote:
>
> > > > > why an array of 8 characters? and why the caller need to set the out parameter?
> > > >
> > > > I'm sorry, this is mistake. It should be long enough to store ip
> > > > address. Thanks for catching that.
> > > > Address argument is just like [ref] - you have to pass a
> > > > pointer to allocated buffer for function to fill in the result.
> > > > Otherwise resolve_name function segfaults.
> > >
> > > I really don't think we want those semantics in libnet.  Why can't
> > > libnet_LookupPdc do the allocation?
> >
> > Sure it can. I just followed what resolve_name required, but I don't
> > mind changing that :)
>
> I'm not entirely sure what you mean here:  Perhaps post your proposed
> change to the list?
I just mean chaning behaviour of libnet_LookupPdc so that it allocates
the buffer and returns it with proper ip address. This means no
requirement to pass allocated buffer to the function as it takes care
of it itself.

It is really quite small change. I don't want to touch resolve_name
part.


cheers,
--
Rafal Szczesniak
Samba Team member  http://www.samba.org


signature.asc (196 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: samba r11705 - in branches/SAMBA_4_0/source/libnet: .

John L.Utz III-2
At Sun, 13 Nov 2005 23:08:30 +0100,
Rafal Szczesniak wrote:

>
> [1  <text/plain; iso-8859-2 (quoted-printable)>]
> On Mon, Nov 14, 2005 at 08:39:51AM +1100, Andrew Bartlett wrote:
> > On Sun, 2005-11-13 at 22:36 +0100, Rafal Szczesniak wrote:
> > > On Mon, Nov 14, 2005 at 08:05:07AM +1100, Andrew Bartlett wrote:
> > > > On Sun, 2005-11-13 at 19:38 +0100, Rafal Szczesniak wrote:
> > > > > On Sun, Nov 13, 2005 at 02:12:56PM +0100, Stefan Metzmacher wrote:
> >
> > > > > > why an array of 8 characters? and why the caller need to set the out parameter?
> > > > >
> > > > > I'm sorry, this is mistake. It should be long enough to store ip
> > > > > address. Thanks for catching that.
> > > > > Address argument is just like [ref] - you have to pass a
> > > > > pointer to allocated buffer for function to fill in the result.
> > > > > Otherwise resolve_name function segfaults.
> > > >
> > > > I really don't think we want those semantics in libnet.  Why can't
> > > > libnet_LookupPdc do the allocation?
> > >
> > > Sure it can. I just followed what resolve_name required, but I don't
> > > mind changing that :)
> >
> > I'm not entirely sure what you mean here:  Perhaps post your proposed
> > change to the list?
>
> I just mean chaning behaviour of libnet_LookupPdc so that it allocates
> the buffer and returns it with proper ip address. This means no
> requirement to pass allocated buffer to the function as it takes care
> of it itself.
>
> It is really quite small change. I don't want to touch resolve_name
> part.

Is there a standard in the samba src tree for buffers like this?

is it traditional to do have the callee do the malloc?

or is it contributor dependent? :-)

My next comment might be incorrect due to my ignorance of some of the
utility functionality in samba pertaining to memory management, but
i'd rather ask the question and be corrected then to just let this
lie:

Wouldnt it be better if the callee's made the callers do the malloc so that
the callers would be less likely to forget to free things?

tnx!


> cheers,
> --
> Rafal Szczesniak
> Samba Team member  http://www.samba.org
>
> [2 Digital signature <application/pgp-signature (7bit)>]
>
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: samba r11705 - in branches/SAMBA_4_0/source/libnet: .

Gerald Carter-4
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

John L.Utz III wrote:

| Wouldnt it be better if the callee's made the callers do the
| malloc so that the callers would be less likely to
| forget to free things?

Hey John. You should probably read up on talloc to get the
full picture for Samba's memory management scheme.

        http://talloc.samba.org/



cheers, jerry
=====================================================================
Alleviating the pain of Windows(tm)      ------- http://www.samba.org
GnuPG Key                ----- http://www.plainjoe.org/gpg_public.asc
"There's an anonymous coward in all of us."               --anonymous
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.0 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFDePQgIR7qMdg1EfYRAlR3AJ4gwNpCeVSAFIr51az+SiiCEMdMOwCeONUj
J1ffJIZMPepGmpt+7AhziqA=
=TG4J
-----END PGP SIGNATURE-----
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: samba r11705 - in branches/SAMBA_4_0/source/libnet: .

John L.Utz III-2
Fair 'nuff!

tnx for the pointer, 'tis why it seemed better to ask then to ponder incorrectly in silence....

At Mon, 14 Nov 2005 14:31:28 -0600,
Gerald (Jerry) Carter wrote:

>
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> John L.Utz III wrote:
>
> | Wouldnt it be better if the callee's made the callers do the
> | malloc so that the callers would be less likely to
> | forget to free things?
>
> Hey John. You should probably read up on talloc to get the
> full picture for Samba's memory management scheme.
>
> http://talloc.samba.org/
>
>
>
> cheers, jerry
> =====================================================================
> Alleviating the pain of Windows(tm)      ------- http://www.samba.org
> GnuPG Key                ----- http://www.plainjoe.org/gpg_public.asc
> "There's an anonymous coward in all of us."               --anonymous
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.0 (GNU/Linux)
> Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org
>
> iD8DBQFDePQgIR7qMdg1EfYRAlR3AJ4gwNpCeVSAFIr51az+SiiCEMdMOwCeONUj
> J1ffJIZMPepGmpt+7AhziqA=
> =TG4J
> -----END PGP SIGNATURE-----
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: samba r11705 - in branches/SAMBA_4_0/source/libnet: .

Rafal Szczesniak
In reply to this post by Gerald Carter-4
On Mon, Nov 14, 2005 at 02:31:28PM -0600, Gerald (Jerry) Carter wrote:

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> John L.Utz III wrote:
>
> | Wouldnt it be better if the callee's made the callers do the
> | malloc so that the callers would be less likely to
> | forget to free things?
>
> Hey John. You should probably read up on talloc to get the
> full picture for Samba's memory management scheme.
Thanks for replying Jerry, I was offline last 24 hours.

John, thanks to talloc used in samba4 all can be cleaned up and freed
when libnet_context is freed (that is assuming you allocate the buffer
using libnet_context as the parent).


cheers,
--
Rafal Szczesniak
Samba Team member  http://www.samba.org


signature.asc (196 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: samba r11705 - in branches/SAMBA_4_0/source/libnet: .

Gerald Carter-4
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Rafal Szczesniak wrote:
>>
>> Hey John. You should probably read up on talloc to get the
>> full picture for Samba's memory management scheme.
>
> Thanks for replying Jerry, I was offline last 24 hours.
>
> John, thanks to talloc used in samba4 all can be cleaned up and freed
> when libnet_context is freed (that is assuming you allocate the buffer
> using libnet_context as the parent).

Just for clarification in the archives....

talloc is not a Samba 4 specific feature.  The talloc lib is
identical in both branches at this point.





cheers, jerry
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.0 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFDegatIR7qMdg1EfYRAr6+AJ4qGSwrgo0Nry7c5x7uN4NlND9awgCgtXok
UPXrxNH+ldQUrkoZcpldAn8=
=I/b/
-----END PGP SIGNATURE-----