Re: [SCM] Samba Shared Repository - branch master updated

[Stefan Metzmacher]

Am 04.01.24 um 15:36 schrieb Brown, James William:

On 1/4/2024 7:46 AM, Stefan Metzmacher wrote:

@@ -3905,7 +3937,24 @@ static int moveip(TALLOC_CTX *mem_ctx, struct 
ctdb_context *ctdb,
 return ret;
 }

-   return 0;
+   /*
+* It isn't strictly necessary to wait until takeover runs are
+* re-enabled but doing so can't hurt.
+*/
+   ret = disable_takeover_runs(mem_ctx,
+   ctdb,
+   0,
+   connected_pnn,
+   connected_count);
+   if (ret != 0) {
+   fprintf(stderr, "Failed to enable takeover runs\n");
+   return ret;
+   }

Message should be "Failed to disable takeover runs\n" like previous change at 
3886.


This is a bit confusing but correct, the function name is strange.

We have "disable" with a timeout of 2*options.timelimit
and "enable" with a timeout of 0.

metze

Re: [SCM] Samba Shared Repository - branch master updated

[Brown, James William]

On 1/4/2024 7:46 AM, Stefan Metzmacher wrote:

@@ -3905,7 +3937,24 @@ static int moveip(TALLOC_CTX *mem_ctx, struct 
ctdb_context *ctdb,
return ret;
}

-   return 0;
+   /*
+* It isn't strictly necessary to wait until takeover runs are
+* re-enabled but doing so can't hurt.
+*/
+   ret = disable_takeover_runs(mem_ctx,
+   ctdb,
+   0,
+   connected_pnn,
+   connected_count);
+   if (ret != 0) {
+   fprintf(stderr, "Failed to enable takeover runs\n");
+   return ret;
+   }

Message should be "Failed to disable takeover runs\n" like previous change at 
3886.

Re: [SCM] Samba Shared Repository - branch master updated

[Stefan Metzmacher]

Am 24.01.23 um 10:16 schrieb Volker Lendecke:

The branch, master has been updated
via  f7b50bc059d smbd: Use smbXsrv_open_global_parse_record() in 
.._verify_record()
via  132b83d0659 smbd: Simplify smbXsrv_open_global_parse_record()
via  2f6776741dc smbd: Move smbXsrv_open_global_parse_record() up in 
smbXsrv_open.c
via  3c779de8cf9 smbd: Simplify smbXsrv_open_global_verify_record()
via  f1a66267bcf smbd: Save a few lines in 
smb2srv_open_lookup_replay_cache()
via  35a32171b50 smbd: Fix a typo
   from  253891032ee python: Don't use deprecated escape sequences

https://git.samba.org/?p=samba.git;a=shortlog;h=master


- Log -
commit f7b50bc059d1b5c7e40cdc4e88ef5ee16f7db670
Author: Volker Lendecke 
Date:   Thu Jan 19 12:29:20 2023 +0100

 smbd: Use smbXsrv_open_global_parse_record() in .._verify_record()
 
 Signed-off-by: Volker Lendecke 
 
 Autobuild-User(master): Volker Lendecke 

 Autobuild-Date(master): Tue Jan 24 09:15:26 UTC 2023 on atb-devel-224

commit 132b83d0659ddc25a96327edc1c7dd23b17a56fd
Author: Volker Lendecke 
Date:   Thu Jan 19 12:25:21 2023 +0100

 smbd: Simplify smbXsrv_open_global_parse_record()
 
 It does not need a db_record.
 
 Signed-off-by: Volker Lendecke 


commit 2f6776741dc6469d78b94da22d75f26a5fc9
Author: Volker Lendecke 
Date:   Thu Jan 19 12:22:33 2023 +0100

 smbd: Move smbXsrv_open_global_parse_record() up in smbXsrv_open.c
 
 Avoid a prototype in the next patches
 
 Signed-off-by: Volker Lendecke 


commit 3c779de8cf99d0936956a12484fd726d5be46c7e
Author: Volker Lendecke 
Date:   Fri Jan 6 16:25:03 2023 +0100

 smbd: Simplify smbXsrv_open_global_verify_record()
 
 Don't depend on the record to be passed in, return NTSTATUS. The two

 flags were a bit confusing to me, now NT_STATUS_OK means "found a
 valid record with a live process", and NT_STATUS_FATAL_APP_EXIT means
 we found a stale record from a crashed smbd
 
 Signed-off-by: Volker Lendecke 


commit f1a66267bcfcd48f3c7ca2ada3f62d40209163e3
Author: Volker Lendecke 
Date:   Wed Jan 11 11:44:29 2023 +0100

 smbd: Save a few lines in smb2srv_open_lookup_replay_cache()
 
 Directly initialize variables, don't leave dangling pointers in TDB_DATA
 
 Signed-off-by: Volker Lendecke 


commit 35a32171b5067d5b80acffc99f8d43cdc7f5f9a7
Author: Volker Lendecke 
Date:   Wed Jan 11 08:18:35 2023 +0100

 smbd: Fix a typo
 
 Signed-off-by: Volker Lendecke 

 Reviewed-by: Stefan Metzmacher 


These were all reviewd by me...

metze

Re: s4:kdc: Set kerberos debug class for kdc service (Re: [SCM] Samba Shared Repository - branch master updated)

[Andreas Schneider]

On Friday, 9 September 2022 10:53:51 CEST Stefan Metzmacher wrote:
> Hi Andreas,
> 
> > - Log -
> > commit 761ce8cfe41139ab5656dec5cc05f2f576095216
> > Author: Andreas Schneider 
> > Date:   Tue Sep 6 10:19:54 2022 +0200
> > 
> >  s4:kdc: Set kerberos debug class for kdc service
> >  
> >  Signed-off-by: Andreas Schneider 
> >  Reviewed-by: Douglas Bagnall 
> >  Reviewed-by: Andrew Bartlett 
> 
> Can we please do this for all files under source4/kdc ?

Hi metze,

yes we can.

https://gitlab.com/samba-team/samba/-/merge_requests/2715

 
> metze


-- 
Andreas Schneider  a...@samba.org
Samba Team www.samba.org
GPG-ID: 8DFF53E18F2ABC8D8F3C92237EE0FC4DCC014E3D

s4:kdc: Set kerberos debug class for kdc service (Re: [SCM] Samba Shared Repository - branch master updated)

[Stefan Metzmacher]

Hi Andreas,


- Log -
commit 761ce8cfe41139ab5656dec5cc05f2f576095216
Author: Andreas Schneider 
Date:   Tue Sep 6 10:19:54 2022 +0200

 s4:kdc: Set kerberos debug class for kdc service
 
 Signed-off-by: Andreas Schneider 

 Reviewed-by: Douglas Bagnall 
 Reviewed-by: Andrew Bartlett 


Can we please do this for all files under source4/kdc ?

metze

Re: [SCM] Samba Shared Repository - branch master updated

[Brown, James William]

The delete should be "rm -f build/DTD/samba.build.pathconfig" to match 
other uses.

On 6/22/2022 12:44 PM, Andreas Schneider wrote:
> diff --git a/docs-xml/Makefile b/docs-xml/Makefile
> index c614097f6c0..ae50b14d79a 100644
> --- a/docs-xml/Makefile
> +++ b/docs-xml/Makefile
> @@ -17,7 +17,7 @@ DBLATEX_OPTIONS = -p xslt/latex.xsl -i xslt/latex
>
>   DATETIME := $(shell date +%Y%m%d%H%M%S)
>
> -XSLTPROC_DEPS = build/catalog.xml build/DTD/samba.build.version
> +XSLTPROC_DEPS = build/catalog.xml build/DTD/samba.build.version 
> build/DTD/samba.build.pathconfig
>
>   ifeq ($(PROFILE), Y)
>   XSLTPROC += --profile --load-trace --timing
> @@ -57,6 +57,7 @@ clean::
>  rm -f xslt/figures/*pdf
>  rm -f $(SMBDOTCONFDOC)/parameters.*.xml
>  rm -f build/catalog.xml
> +   rm -f DTD/samba.build.pathconfig

Re: Conflicting public symbols... (Re: [SCM] Samba Shared Repository - branch master updated)

[Jeremy Allison]

On Wed, Feb 10, 2021 at 10:36:18PM +0100, Stefan Metzmacher wrote:

Hi,


- Log -
commit 551532d007970ab11dca71b532754728a6a78496
Author: xzhao9 
Date:   Sun Jan 3 13:02:18 2021 -0500

s3:registry Renaming get_charset() to smbreg_get_charset()

Rename to smbreg_get_charset() function to avoid naming conflict
with MariaDB.

Signed-off-by: xzhao9 
Reviewed-by: Jeremy Allison 
Reviewed-by: David Mulder 

Autobuild-User(master): Jeremy Allison 
Autobuild-Date(master): Wed Feb 10 21:00:28 UTC 2021 on sn-devel-184


Do we need a bug report to backport this?


I'll create one.


There's a similar problem reported here:
https://www.downtowndougbrown.com/2021/01/tracking-down-a-segfault-that-suddenly-started-happening/

Does anyone have time to have a deeper look?


Looks like "-fvisibility=hidden" is the way
to go here.

Conflicting public symbols... (Re: [SCM] Samba Shared Repository - branch master updated)

[Stefan Metzmacher]

Hi,

> - Log -
> commit 551532d007970ab11dca71b532754728a6a78496
> Author: xzhao9 
> Date:   Sun Jan 3 13:02:18 2021 -0500
> 
> s3:registry Renaming get_charset() to smbreg_get_charset()
> 
> Rename to smbreg_get_charset() function to avoid naming conflict
> with MariaDB.
> 
> Signed-off-by: xzhao9 
> Reviewed-by: Jeremy Allison 
> Reviewed-by: David Mulder 
> 
> Autobuild-User(master): Jeremy Allison 
> Autobuild-Date(master): Wed Feb 10 21:00:28 UTC 2021 on sn-devel-184

Do we need a bug report to backport this?

There's a similar problem reported here:
https://www.downtowndougbrown.com/2021/01/tracking-down-a-segfault-that-suddenly-started-happening/

Does anyone have time to have a deeper look?

Thanks!
metze



signature.asc
Description: OpenPGP digital signature

Re: cli_credentials_parse_name... (Re: [SCM] Samba Shared Repository - branch master updated)

[Alexander Bokovoy]

On to, 05 marras 2020, Andrew Bartlett wrote:
> On Wed, 2020-11-04 at 19:23 +0200, Alexander Bokovoy wrote:
> > On ke, 04 marras 2020, Stefan Metzmacher wrote:
> > > Am 04.11.20 um 17:24 schrieb Alexander Bokovoy:
> > > > The branch, master has been updated
> > > >via  f9016912098 lookup_name: allow lookup for own realm
> > > >via  00f4262ed0b cli_credentials: add a helper to parse
> > > > user or group names
> > > >via  eb0474d27ba cli_credentials_parse_string: fix parsing
> > > > of principals
> > > >   from  a1b021200e3 selftest: add test for new "samba-tool
> > > > user unlock" command
> > > > 
> > > > https://git.samba.org/?p=samba.git;a=shortlog;h=master
> > > > 
> > > > 
> > > > - Log -
> > > > 
> > > > commit f901691209867b32c2d7c5c9274eee196f541654
> > > > Author: Alexander Bokovoy 
> > > > Date:   Wed Nov 4 14:21:33 2020 +0200
> > > > 
> > > > lookup_name: allow lookup for own realm
> > > > 
> > > > When using a security tab in Windows Explorer, a lookup over
> > > > a trusted
> > > > forest might come as realm\name instead of NetBIOS domain
> > > > name:
> > > > 
> > > > -
> > > > ---
> > > > [2020/01/13 11:12:39.859134,  1, pid=33253,
> > > > effective(1732401004, 1732401004), real(1732401004, 0),
> > > > class=rpc_parse]
> > > > ../../librpc/ndr/ndr.c:471(ndr_print_function_debug)
> > > >lsa_LookupNames3: struct lsa_LookupNames3
> > > >   in: struct lsa_LookupNames3
> > > >   handle   : *
> > > >   handle: struct policy_handle
> > > >   handle_type  : 0x
> > > > (0)
> > > >   uuid : 000e-
> > > > --1c5e-a750e581
> > > >   num_names: 0x0001 (1)
> > > >   names: ARRAY(1)
> > > >   names: struct lsa_String
> > > >   length   : 0x001e (30)
> > > >   size : 0x0020 (32)
> > > >   string   : *
> > > >   string   :
> > > > 'ipa.test\admins'
> > > >   sids : *
> > > >   sids: struct lsa_TransSidArray3
> > > >   count: 0x
> > > > (0)
> > > >   sids : NULL
> > > >   level:
> > > > LSA_LOOKUP_NAMES_UPLEVEL_TRUSTS_ONLY2 (6)
> > > >   count: *
> > > >   count: 0x (0)
> > > >   lookup_options   :
> > > > LSA_LOOKUP_OPTION_SEARCH_ISOLATED_NAMES (0)
> > > >   client_revision  :
> > > > LSA_CLIENT_REVISION_2 (2)
> > > > 
> > > > ...
> > > > 
> > > > diff --git a/auth/credentials/tests/test_creds.c
> > > > b/auth/credentials/tests/test_creds.c
> > > > index d2d3d30d73d..38550d6ecf9 100644
> > > > --- a/auth/credentials/tests/test_creds.c
> > > > +++ b/auth/credentials/tests/test_creds.c
> > > > @@ -187,7 +187,7 @@ static void torture_creds_parse_string(void
> > > > **state)
> > > > assert_string_equal(creds->domain, "");
> > > > assert_int_equal(creds->domain_obtained, CRED_SPECIFIED);
> > > >  
> > > > -   assert_string_equal(creds->username, "wurst@brot.realm");
> > > > +   assert_string_equal(creds->username, "wurst");
> > > 
> > > I'm sorry but this is wrong!
> > > I'm wondering why this wasn't covered by any high level test.
> > > 
> > > This needs to result in domain="" and username="wurst@brot.realm"
> > > and that's exactly what we need to use for NTLMSSP.
> > > Also note that "brot.realm" may not be a realm and "wurst" may not
> > > be a sAMAccountName. A userPrincipalName can be 
> > > anything@anydomain-of-msDS-SPNSuffixes.
> 
> cli_credentials_get_ntlm_username_domain() does this already.
> 
> > > I fear we need to revert these changes.
> > > From the merge request (
> > > https://gitlab.com/samba-team/samba/-/merge_requests/1658)
> > > I didn't really look at the whole patchset (with behavior change)
> > > I only focused on CRED_NO_PASSWORD.
> > > 
> > > I think we need to logic we have in wb_irpc_lsa_LookupNames4_call()
> > > and/or parse_domain_user() here.

One thing I need for lookup_name() is to be able to lookup those
realm/NetBIOS-qualified requests along with UPN-based queries through
passdb when they are done with LookupNames3 using
LSA_LOOKUP_NAMES_UPLEVEL_TRUSTS_ONLY2 level. We don't see the level
passed, as lookup_name() operates on own flags only.  Without
lookup_name() on DC (FreeIPA DC) being able to lookup UPNs against
passdb, we don't even get the reque

Re: cli_credentials_parse_name... (Re: [SCM] Samba Shared Repository - branch master updated)

[Andrew Bartlett]

On Wed, 2020-11-04 at 19:23 +0200, Alexander Bokovoy wrote:
> On ke, 04 marras 2020, Stefan Metzmacher wrote:
> > Am 04.11.20 um 17:24 schrieb Alexander Bokovoy:
> > > The branch, master has been updated
> > >via  f9016912098 lookup_name: allow lookup for own realm
> > >via  00f4262ed0b cli_credentials: add a helper to parse
> > > user or group names
> > >via  eb0474d27ba cli_credentials_parse_string: fix parsing
> > > of principals
> > >   from  a1b021200e3 selftest: add test for new "samba-tool
> > > user unlock" command
> > > 
> > > https://git.samba.org/?p=samba.git;a=shortlog;h=master
> > > 
> > > 
> > > - Log -
> > > 
> > > commit f901691209867b32c2d7c5c9274eee196f541654
> > > Author: Alexander Bokovoy 
> > > Date:   Wed Nov 4 14:21:33 2020 +0200
> > > 
> > > lookup_name: allow lookup for own realm
> > > 
> > > When using a security tab in Windows Explorer, a lookup over
> > > a trusted
> > > forest might come as realm\name instead of NetBIOS domain
> > > name:
> > > 
> > > -
> > > ---
> > > [2020/01/13 11:12:39.859134,  1, pid=33253,
> > > effective(1732401004, 1732401004), real(1732401004, 0),
> > > class=rpc_parse]
> > > ../../librpc/ndr/ndr.c:471(ndr_print_function_debug)
> > >lsa_LookupNames3: struct lsa_LookupNames3
> > >   in: struct lsa_LookupNames3
> > >   handle   : *
> > >   handle: struct policy_handle
> > >   handle_type  : 0x
> > > (0)
> > >   uuid : 000e-
> > > --1c5e-a750e581
> > >   num_names: 0x0001 (1)
> > >   names: ARRAY(1)
> > >   names: struct lsa_String
> > >   length   : 0x001e (30)
> > >   size : 0x0020 (32)
> > >   string   : *
> > >   string   :
> > > 'ipa.test\admins'
> > >   sids : *
> > >   sids: struct lsa_TransSidArray3
> > >   count: 0x
> > > (0)
> > >   sids : NULL
> > >   level:
> > > LSA_LOOKUP_NAMES_UPLEVEL_TRUSTS_ONLY2 (6)
> > >   count: *
> > >   count: 0x (0)
> > >   lookup_options   :
> > > LSA_LOOKUP_OPTION_SEARCH_ISOLATED_NAMES (0)
> > >   client_revision  :
> > > LSA_CLIENT_REVISION_2 (2)
> > > 
> > > ...
> > > 
> > > diff --git a/auth/credentials/tests/test_creds.c
> > > b/auth/credentials/tests/test_creds.c
> > > index d2d3d30d73d..38550d6ecf9 100644
> > > --- a/auth/credentials/tests/test_creds.c
> > > +++ b/auth/credentials/tests/test_creds.c
> > > @@ -187,7 +187,7 @@ static void torture_creds_parse_string(void
> > > **state)
> > >   assert_string_equal(creds->domain, "");
> > >   assert_int_equal(creds->domain_obtained, CRED_SPECIFIED);
> > >  
> > > - assert_string_equal(creds->username, "wurst@brot.realm");
> > > + assert_string_equal(creds->username, "wurst");
> > 
> > I'm sorry but this is wrong!
> > I'm wondering why this wasn't covered by any high level test.
> > 
> > This needs to result in domain="" and username="wurst@brot.realm"
> > and that's exactly what we need to use for NTLMSSP.
> > Also note that "brot.realm" may not be a realm and "wurst" may not
> > be a sAMAccountName. A userPrincipalName can be 
> > anything@anydomain-of-msDS-SPNSuffixes.

cli_credentials_get_ntlm_username_domain() does this already.

> > I fear we need to revert these changes.
> > From the merge request (
> > https://gitlab.com/samba-team/samba/-/merge_requests/1658)
> > I didn't really look at the whole patchset (with behavior change)
> > I only focused on CRED_NO_PASSWORD.
> > 
> > I think we need to logic we have in wb_irpc_lsa_LookupNames4_call()
> > and/or parse_domain_user() here.
> 
> I'm pushing a revert for now and will look at those.

I'm not so sure this is totally wrong.  Can I have a look over these
paths at the office?  I need any possible distraction from US election
results anyway...

Andrew Bartlett

-- 
Andrew Bartlett   https://samba.org/~abartlet/
Authentication Developer, Samba Team  https://samba.org
Samba Developer, Catalyst IT  
https://catalyst.net.nz/services/samba

Re: cli_credentials_parse_name... (Re: [SCM] Samba Shared Repository - branch master updated)

[Alexander Bokovoy]

On ke, 04 marras 2020, Stefan Metzmacher wrote:
> Am 04.11.20 um 17:24 schrieb Alexander Bokovoy:
> > The branch, master has been updated
> >via  f9016912098 lookup_name: allow lookup for own realm
> >via  00f4262ed0b cli_credentials: add a helper to parse user or 
> > group names
> >via  eb0474d27ba cli_credentials_parse_string: fix parsing of 
> > principals
> >   from  a1b021200e3 selftest: add test for new "samba-tool user unlock" 
> > command
> > 
> > https://git.samba.org/?p=samba.git;a=shortlog;h=master
> > 
> > 
> > - Log -
> > commit f901691209867b32c2d7c5c9274eee196f541654
> > Author: Alexander Bokovoy 
> > Date:   Wed Nov 4 14:21:33 2020 +0200
> > 
> > lookup_name: allow lookup for own realm
> > 
> > When using a security tab in Windows Explorer, a lookup over a trusted
> > forest might come as realm\name instead of NetBIOS domain name:
> > 
> > 
> > [2020/01/13 11:12:39.859134,  1, pid=33253, effective(1732401004, 
> > 1732401004), real(1732401004, 0), class=rpc_parse] 
> > ../../librpc/ndr/ndr.c:471(ndr_print_function_debug)
> >lsa_LookupNames3: struct lsa_LookupNames3
> >   in: struct lsa_LookupNames3
> >   handle   : *
> >   handle: struct policy_handle
> >   handle_type  : 0x (0)
> >   uuid : 
> > 000e---1c5e-a750e581
> >   num_names: 0x0001 (1)
> >   names: ARRAY(1)
> >   names: struct lsa_String
> >   length   : 0x001e (30)
> >   size : 0x0020 (32)
> >   string   : *
> >   string   : 'ipa.test\admins'
> >   sids : *
> >   sids: struct lsa_TransSidArray3
> >   count: 0x (0)
> >   sids : NULL
> >   level: 
> > LSA_LOOKUP_NAMES_UPLEVEL_TRUSTS_ONLY2 (6)
> >   count: *
> >   count: 0x (0)
> >   lookup_options   : 
> > LSA_LOOKUP_OPTION_SEARCH_ISOLATED_NAMES (0)
> >   client_revision  : LSA_CLIENT_REVISION_2 (2)
> >
> > ...
> >
> > diff --git a/auth/credentials/tests/test_creds.c 
> > b/auth/credentials/tests/test_creds.c
> > index d2d3d30d73d..38550d6ecf9 100644
> > --- a/auth/credentials/tests/test_creds.c
> > +++ b/auth/credentials/tests/test_creds.c
> > @@ -187,7 +187,7 @@ static void torture_creds_parse_string(void **state)
> > assert_string_equal(creds->domain, "");
> > assert_int_equal(creds->domain_obtained, CRED_SPECIFIED);
> >  
> > -   assert_string_equal(creds->username, "wurst@brot.realm");
> > +   assert_string_equal(creds->username, "wurst");
> 
> I'm sorry but this is wrong!
> I'm wondering why this wasn't covered by any high level test.
> 
> This needs to result in domain="" and username="wurst@brot.realm"
> and that's exactly what we need to use for NTLMSSP.
> Also note that "brot.realm" may not be a realm and "wurst" may not
> be a sAMAccountName. A userPrincipalName can be 
> anything@anydomain-of-msDS-SPNSuffixes.
> 
> I fear we need to revert these changes.
> From the merge request 
> (https://gitlab.com/samba-team/samba/-/merge_requests/1658)
> I didn't really look at the whole patchset (with behavior change)
> I only focused on CRED_NO_PASSWORD.
> 
> I think we need to logic we have in wb_irpc_lsa_LookupNames4_call() and/or 
> parse_domain_user() here.

I'm pushing a revert for now and will look at those.


-- 
/ Alexander Bokovoy

cli_credentials_parse_name... (Re: [SCM] Samba Shared Repository - branch master updated)

[Stefan Metzmacher]

Am 04.11.20 um 17:24 schrieb Alexander Bokovoy:
> The branch, master has been updated
>via  f9016912098 lookup_name: allow lookup for own realm
>via  00f4262ed0b cli_credentials: add a helper to parse user or group 
> names
>via  eb0474d27ba cli_credentials_parse_string: fix parsing of 
> principals
>   from  a1b021200e3 selftest: add test for new "samba-tool user unlock" 
> command
> 
> https://git.samba.org/?p=samba.git;a=shortlog;h=master
> 
> 
> - Log -
> commit f901691209867b32c2d7c5c9274eee196f541654
> Author: Alexander Bokovoy 
> Date:   Wed Nov 4 14:21:33 2020 +0200
> 
> lookup_name: allow lookup for own realm
> 
> When using a security tab in Windows Explorer, a lookup over a trusted
> forest might come as realm\name instead of NetBIOS domain name:
> 
> 
> [2020/01/13 11:12:39.859134,  1, pid=33253, effective(1732401004, 
> 1732401004), real(1732401004, 0), class=rpc_parse] 
> ../../librpc/ndr/ndr.c:471(ndr_print_function_debug)
>lsa_LookupNames3: struct lsa_LookupNames3
>   in: struct lsa_LookupNames3
>   handle   : *
>   handle: struct policy_handle
>   handle_type  : 0x (0)
>   uuid : 
> 000e---1c5e-a750e581
>   num_names: 0x0001 (1)
>   names: ARRAY(1)
>   names: struct lsa_String
>   length   : 0x001e (30)
>   size : 0x0020 (32)
>   string   : *
>   string   : 'ipa.test\admins'
>   sids : *
>   sids: struct lsa_TransSidArray3
>   count: 0x (0)
>   sids : NULL
>   level: 
> LSA_LOOKUP_NAMES_UPLEVEL_TRUSTS_ONLY2 (6)
>   count: *
>   count: 0x (0)
>   lookup_options   : 
> LSA_LOOKUP_OPTION_SEARCH_ISOLATED_NAMES (0)
>   client_revision  : LSA_CLIENT_REVISION_2 (2)
>
> ...
>
> diff --git a/auth/credentials/tests/test_creds.c 
> b/auth/credentials/tests/test_creds.c
> index d2d3d30d73d..38550d6ecf9 100644
> --- a/auth/credentials/tests/test_creds.c
> +++ b/auth/credentials/tests/test_creds.c
> @@ -187,7 +187,7 @@ static void torture_creds_parse_string(void **state)
>   assert_string_equal(creds->domain, "");
>   assert_int_equal(creds->domain_obtained, CRED_SPECIFIED);
>  
> - assert_string_equal(creds->username, "wurst@brot.realm");
> + assert_string_equal(creds->username, "wurst");

I'm sorry but this is wrong!
I'm wondering why this wasn't covered by any high level test.

This needs to result in domain="" and username="wurst@brot.realm"
and that's exactly what we need to use for NTLMSSP.
Also note that "brot.realm" may not be a realm and "wurst" may not
be a sAMAccountName. A userPrincipalName can be 
anything@anydomain-of-msDS-SPNSuffixes.

I fear we need to revert these changes.
From the merge request 
(https://gitlab.com/samba-team/samba/-/merge_requests/1658)
I didn't really look at the whole patchset (with behavior change)
I only focused on CRED_NO_PASSWORD.

I think we need to logic we have in wb_irpc_lsa_LookupNames4_call() and/or 
parse_domain_user() here.

metze



signature.asc
Description: OpenPGP digital signature

Re: [SCM] Samba Shared Repository - branch master updated

[jim]

Amitay

You did not add "|| true" to the statd-callout invocation of nfsconf 
like  you did in nfs-linux-kernel-callout.


Regards,
Jim

On 7/27/2020 3:07 AM, Amitay Isaacs wrote:

+# If NFS_HOSTNAME not set then try to pull it out of /etc/nfs.conf
+if [ -z "$NFS_HOSTNAME" ] && type nfsconf >/dev/null 2>&1 ; then
+   NFS_HOSTNAME=$(nfsconf --get statd name)
+fi

ctdb_pkt_recv_recv regression (Re: [SCM] Samba Shared Repository - branch master updated)

[Stefan Metzmacher]

Hi Noel,

can you please have a look at the mail I wrote a month ago,
I think we need to fix that up.

Thanks!
metze

Am 26.07.19 um 21:31 schrieb Stefan Metzmacher:
> Hi Noel,
> 
> thanks for fixing clang warnings, but
> I fear the following changes are wrong.
> 
> Can we have SMB_ASSERT(hdr != NULL); after the if statements with a
> comment explaining why it's there.
> 
>> diff --git a/source3/lib/ctdbd_conn.c b/source3/lib/ctdbd_conn.c
>> index d5fac572d3c..8a8fbec4552 100644
>> --- a/source3/lib/ctdbd_conn.c
>> +++ b/source3/lib/ctdbd_conn.c
>> @@ -398,15 +398,16 @@ static int ctdb_read_packet(int fd, int timeout, 
>> TALLOC_CTX *mem_ctx,
>>  static int ctdb_read_req(struct ctdbd_connection *conn, uint32_t reqid,
>>   TALLOC_CTX *mem_ctx, struct ctdb_req_header **result)
>>  {
>> -struct ctdb_req_header *hdr;
>> +struct ctdb_req_header *hdr = NULL;
>>  int ret;
>>  
>>   next_pkt:
>>  
>>  ret = ctdb_read_packet(conn->fd, conn->timeout, mem_ctx, &hdr);
>> -if (ret != 0) {
>> +if (hdr == NULL || ret != 0) {
>>  DBG_ERR("ctdb_read_packet failed: %s\n", strerror(ret));
>>  cluster_fatal("failed to read data from ctdbd\n");
>> +return -1;
>>  }
> 
> This is not strictly a bug, but it's makes the code really unclear.
> 
>>  DEBUG(11, ("Received ctdb packet\n"));
>> @@ -1038,7 +1039,7 @@ int ctdbd_traverse(struct ctdbd_connection *conn, 
>> uint32_t db_id,
>>  int ret;
>>  TDB_DATA key, data;
>>  struct ctdb_traverse_start t;
>> -int32_t cstatus;
>> +int32_t cstatus = 0;
>>  
>>  if (ctdbd_conn_has_async_reqs(conn)) {
>>  /*
>> @@ -1945,7 +1946,7 @@ static void ctdbd_parse_done(struct tevent_req *subreq)
>>  
>>  ret = ctdb_pkt_recv_recv(subreq, state, &hdr);
>>  TALLOC_FREE(subreq);
>> -if (tevent_req_error(req, ret)) {
>> +if ((hdr == NULL) || tevent_req_error(req, ret)) {
>>  DBG_ERR("ctdb_pkt_recv_recv returned %s\n", strerror(ret));
>>  return;
>>  }
> 
> This is actually really a bug, in case  ctdb_pkt_recv_recv fails
> we'll never run tevent_req_error() and the callers callback function,
> so it will just hang as hdr is always NULL if ret is not 0.
> 
> Can you revert this and add SMB_ASSERT().
> 
> Thanks!
> metze
> 




signature.asc
Description: OpenPGP digital signature

Re: [SCM] Samba Shared Repository - branch master updated

[Stefan Metzmacher]

Hi Noel,

thanks for fixing clang warnings, but
I fear the following changes are wrong.

Can we have SMB_ASSERT(hdr != NULL); after the if statements with a
comment explaining why it's there.

> diff --git a/source3/lib/ctdbd_conn.c b/source3/lib/ctdbd_conn.c
> index d5fac572d3c..8a8fbec4552 100644
> --- a/source3/lib/ctdbd_conn.c
> +++ b/source3/lib/ctdbd_conn.c
> @@ -398,15 +398,16 @@ static int ctdb_read_packet(int fd, int timeout, 
> TALLOC_CTX *mem_ctx,
>  static int ctdb_read_req(struct ctdbd_connection *conn, uint32_t reqid,
>TALLOC_CTX *mem_ctx, struct ctdb_req_header **result)
>  {
> - struct ctdb_req_header *hdr;
> + struct ctdb_req_header *hdr = NULL;
>   int ret;
>  
>   next_pkt:
>  
>   ret = ctdb_read_packet(conn->fd, conn->timeout, mem_ctx, &hdr);
> - if (ret != 0) {
> + if (hdr == NULL || ret != 0) {
>   DBG_ERR("ctdb_read_packet failed: %s\n", strerror(ret));
>   cluster_fatal("failed to read data from ctdbd\n");
> + return -1;
>   }

This is not strictly a bug, but it's makes the code really unclear.

>   DEBUG(11, ("Received ctdb packet\n"));
> @@ -1038,7 +1039,7 @@ int ctdbd_traverse(struct ctdbd_connection *conn, 
> uint32_t db_id,
>   int ret;
>   TDB_DATA key, data;
>   struct ctdb_traverse_start t;
> - int32_t cstatus;
> + int32_t cstatus = 0;
>  
>   if (ctdbd_conn_has_async_reqs(conn)) {
>   /*
> @@ -1945,7 +1946,7 @@ static void ctdbd_parse_done(struct tevent_req *subreq)
>  
>   ret = ctdb_pkt_recv_recv(subreq, state, &hdr);
>   TALLOC_FREE(subreq);
> - if (tevent_req_error(req, ret)) {
> + if ((hdr == NULL) || tevent_req_error(req, ret)) {
>   DBG_ERR("ctdb_pkt_recv_recv returned %s\n", strerror(ret));
>   return;
>   }

This is actually really a bug, in case  ctdb_pkt_recv_recv fails
we'll never run tevent_req_error() and the callers callback function,
so it will just hang as hdr is always NULL if ret is not 0.

Can you revert this and add SMB_ASSERT().

Thanks!
metze



signature.asc
Description: OpenPGP digital signature

sambadowngradedatabase (Re: [SCM] Samba Shared Repository - branch master updated)

[Stefan Metzmacher]

Hi Andrew,

can you please prepare backports of the sambadowngradedatabase changes?
These are needed in the old branches.

Thanks!
metze

Am 29.05.19 um 07:58 schrieb Andrew Bartlett:
> The branch, master has been updated
>via  8831b06d3d4 ldb: Release ldb 2.0.3
>via  4a95410a154 downgradedatabase: blackbox: MDB backend
>via  0942a65b63c downgradedatabase: adding special case for MDB
>via  4eee09a2c17 dsdb: disable ORDERED_INTEGER with MDB pack format v1
>via  6b4abb99521 ldb: pack_format_override option
>via  68d99187cd5 downgradedatabase: blackbox: database repacked
>via  8db1312b08e ldb: python test for repack
>via  73763acf49c ldb: repack old format database if GUID indexing 
> enabled
>via  d6ded22cb61 downgradedatabase: blackbox: check ordered integer 
> removed
>via  08b9d204b6e ldb: binding ordered indexes to GUID indexing
>via  6005c8cbad7 ldb: only used a->syntax->index_format_fn if GUID 
> indexing is enabled
>via  74d15c9bf76 downgradedatabase: blackbox test
>via  09f2a187b3d sambadowngradedatabase: Add "or later" to warning 
> about using tools from Samba 4.8
>via  c0b679f6a3f sambaundoguididx: renamed to downgradedatabase
>via  40ca8ed5a15 sambaundoguididx: fix for -s
>via  a3101b9704f ldb: Fix segfault parsing new pack formats
>via  2de0aebed60 ldb: test for parse errors
>   from  73080f255e3 smbd: Adapt brl_pending_overlap to README.Coding


> https://git.samba.org/?p=samba.git;a=shortlog;h=master
> 
> 
> - Log -
> commit 8831b06d3d4c1cb5b7732e9863228f8f3aea4e36
> Author: Aaron Haslett 
> Date:   Fri May 24 15:34:43 2019 +1200
> 
> ldb: Release ldb 2.0.3
> 
> * Default pack format changed to version 2 for GUID Indexed databases
>   (essentially Samba's sam.ldb). This means faster unpacking for records
>   with lots of attributes.
> 
> * Enforce link between all new database features, following toggling of
>   GUID indexing. If user toggles GUID indexing on/off, target pack format
>   is changed. Likewise the special ORDERED_INTEGER index format is used
>   (when GUID indexing on) or behaves like INTEGER (when GUID indexing 
> off).
> 
> * Database repacked if packing format not as expected.
> 
> * In the case of MDB, since GUID indexing is mandatory, feature toggling
>   provided by pack_format_override LDB option.
> 
> * Check for errors from ldb_unpack_data() in ldb_tdb
> 
> Signed-off-by: Aaron Haslett 
> Signed-off-by: Andrew Bartlett 
> Reviewed-by: Garming Sam 
> Pair-Programmed-With: Andrew Bartlett 
> 
> Autobuild-User(master): Andrew Bartlett 
> Autobuild-Date(master): Wed May 29 05:56:59 UTC 2019 on sn-devel-184
> 
> commit 4a95410a1543f854743aac24c6c7dc5ffc423aae
> Author: Aaron Haslett 
> Date:   Fri May 24 14:54:03 2019 +1200
> 
> downgradedatabase: blackbox: MDB backend
> 
> When Samba uses TDB as a backend store, all new database features are
> toggled on/off when GUID indexing is toggled. But, GUID indexing can't
> be disabled on MDB, so the other features are toggled separately.
> Consequently, the downgradedatabase script behaves differently depending
> on the database backend. This patch adds testing for the MDB behaviour.
> 
> Signed-off-by: Aaron Haslett 
> Signed-off-by: Andrew Bartlett 
> Reviewed-by: Garming Sam 
> Pair-Programmed-With: Andrew Bartlett 
> 
> commit 0942a65b63cc99f36d3eba99e9c9551e10c5782e
> Author: Aaron Haslett 
> Date:   Thu May 23 20:06:56 2019 +1200
> 
> downgradedatabase: adding special case for MDB
> 
> Though this script was initially written for undoing GUID indexing on
> TDB databases, we're repurposing it to do a full downgrade of any
> database. MDB databases can't be DN indexed, but they can have pack
> format version 2 and ORDERED_INTEGER data types, which must be removed
> during a downgrade.
> 
> Signed-off-by: Aaron Haslett 
> Signed-off-by: Andrew Bartlett 
> Reviewed-by: Garming Sam 
> Pair-Programmed-With: Andrew Bartlett 
> 
> commit 4eee09a2c17d1276b1d0be9f26b23743eec485c2
> Author: Aaron Haslett 
> Date:   Thu May 23 20:13:05 2019 +1200
> 
> dsdb: disable ORDERED_INTEGER with MDB pack format v1
> 
> For TDB databases, the new ORDERED_INTEGER type is disabled along with
> repacking at format version 1 if GUID indexing is disabled, so all the new
> database features are toggled together. This scheme doesn't work with
> MDB because GUID indexing is mandatory when using MDB. However, a
> downgrade path is still required so in a previous commit we added
> a pack_format_override option which allows a downgrade script to force
> the database to use an earlier packing format. But, the new
> ORDERED_INTEGE

Re: [SCM] Samba Shared Repository - branch master updated

[Andrew Bartlett]

On Thu, 2019-03-07 at 05:59 +0100, Andrew Bartlett wrote:
> The branch, master has been updated
>via  8b18da27cf2 s4-server: Open and close a transaction on sam.ldb at 
> startup
>via  6f2558cab81 dsdb: Provide better error strings in rootdse GUID 
> attribute handling
>via  30f93bc7ac7 kcc: Give a better error message when 
> samdb_ntds_objectGUID fails
>via  0c52a6bee78 dsdb: Unify 
> samdb_{get,set}_ntds_{objectGUID,invocation_id}

My apologies, the last three commits were reviewed by Douglas here:
https://gitlab.com/samba-team/samba/merge_requests/273#note_148039507

Sorry,

Andrew Bartlett

-- 
Andrew Bartlett   http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT  http://catalyst.net.nz/services/samba

Re: [SCM] Samba Shared Repository - branch master updated

[Tim Beale]

Hi Metze,

I wanted to rename the python module because the naming didn't seem
consistent with any of the other python bindings Samba has. I think
'libsmb_samba_internal' made sense when it was unused by any of the
samba python code (except some test code). But now (well, soon) these
Python bindings will be used by the samba-tool code. It looks a bit
strange in samba-tool to have:

conn = libsmb_samba_internal.Conn(server)

E.g. my latest patches start to use the new bindings in more places:
https://gitlab.com/catalyst-samba/samba/commits/timb-pysmb-latest

But maybe I've misunderstood something. Why do you want to keep the
libsmb_samba_internal name?

Cheers,
Tim

On 8/01/19 5:54 PM, Stefan Metzmacher wrote:
> Hi Tim,
>
>> diff --git a/source3/wscript_build b/source3/wscript_build
>> index a8ea8e581df..9d188a8d36a 100644
>> --- a/source3/wscript_build
>> +++ b/source3/wscript_build
>> @@ -1323,7 +1323,7 @@ for env in bld.gen_python_environments():
>>  bld.SAMBA3_PYTHON('pylibsmb',
>>source='libsmb/pylibsmb.c',
>>deps='smbclient samba-credentials %s' % pycredentials,
>> -  realname='samba/samba3/libsmb_samba_internal.so'
>> +  realname='samba/samba3/libsmb.so'
>>)
> What's wrong with libsmb_samba_internal? I'd keep the name.
>
> metze
>

Re: [SCM] Samba Shared Repository - branch master updated

[Stefan Metzmacher]

Hi Tim,

> diff --git a/source3/wscript_build b/source3/wscript_build
> index a8ea8e581df..9d188a8d36a 100644
> --- a/source3/wscript_build
> +++ b/source3/wscript_build
> @@ -1323,7 +1323,7 @@ for env in bld.gen_python_environments():
>  bld.SAMBA3_PYTHON('pylibsmb',
>source='libsmb/pylibsmb.c',
>deps='smbclient samba-credentials %s' % pycredentials,
> -  realname='samba/samba3/libsmb_samba_internal.so'
> +  realname='samba/samba3/libsmb.so'
>)

What's wrong with libsmb_samba_internal? I'd keep the name.

metze



signature.asc
Description: OpenPGP digital signature

Re: [SCM] Samba Shared Repository - branch master updated

[Alexander Bokovoy]

On pe, 07 syys 2018, Andrew Bartlett wrote:
> The branch, master has been updated
>via  4847daf wafsamba/samba_waf18: redefine flex function
>via  364077d lib/replace/wscript: fix detection of a fallthrough 
> attribute for clang
>via  311e1eb wafsamba/samba_autoconf: when setting undefined result, 
> use empty tuple
>via  193fdbf s3/wscript: fix flex detection
>via  e761271 s3/wscript: fix bison detection
>   from  4a63ab9 s4/selftest/tests: Enabled samba.tests.samba_tool.computer
> 
> https://git.samba.org/?p=samba.git;a=shortlog;h=master
While Andrew pushed the current patch set, Metze pointed out that it
didn't solve the issue with CONFIG_SET() still reporting undefined
defines are set if they were stored in the cache as '0'.

Attached patch should fix the issue. On FreeBSD 11.2 I now have:

$ egrep '(HAVE_BSD_STRTOLL|INCOHERENT_MMAP|WORKING_STRPTIME)' 
bin/default/include/config.h bin/c4che/default_cache.py
bin/default/include/config.h:#define HAVE_BSD_STRTOLL 1
bin/default/include/config.h:/* #undef HAVE_WORKING_STRPTIME */
bin/default/include/config.h:/* #undef HAVE_INCOHERENT_MMAP */
bin/c4che/default_cache.py:HAVE_BSD_STRTOLL = 1
bin/c4che/default_cache.py:HAVE_INCOHERENT_MMAP = ()
bin/c4che/default_cache.py:HAVE_WORKING_STRPTIME = ()

-- 
/ Alexander Bokovoy
>From 92f2b9b2bac513412169de959285c35a40ea9d07 Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy 
Date: Fri, 7 Sep 2018 14:34:48 +
Subject: [PATCH] buildtools/wafsamba: undefine a define for a failed test
 unless asked to keep it

If conf.CHECK_CODE() is called without `always=True` and the test has
failed, undefine the define already set to '0' by conf.check_code().

This restores expectations that undefined symbols are not considered to
be set by CONFIG_SET() method.

Signed-off-by: Alexander Bokovoy 
---
 buildtools/wafsamba/samba_autoconf.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/buildtools/wafsamba/samba_autoconf.py 
b/buildtools/wafsamba/samba_autoconf.py
index 6b940e53c00..ced824d593d 100644
--- a/buildtools/wafsamba/samba_autoconf.py
+++ b/buildtools/wafsamba/samba_autoconf.py
@@ -447,6 +447,8 @@ def CHECK_CODE(conf, code, define,
 except Exception:
 if always:
 conf.DEFINE(define, 0)
+else:
+conf.undefine(define)
 conf.COMPOUND_END(False)
 if mandatory:
 raise
-- 
2.18.0

Re: [SCM] Samba Shared Repository - branch master updated

[Alexander Bokovoy]

On ke, 05 syys 2018, Amitay Isaacs wrote:
> > Thank you, Andrew, for the reviews and persistent push to complete this!
> >
> > We are using WAF 2.x now which should allow us to concentrate on
> > switching to Python 3 for the build process as well for all our
> > libraries and components.
> >
> > If you see any build issues related to the WAF, please report them.
> > Autobuild succeeded but we might still have pieces of Python code that
> > aren't excercised through the autobuild targets.
> 
> Great job updating to the latest waf.
> 
> Now fixing all the corner cases... ;-)
:)

> 
> On freebsd-11, waf does not want to use clang and complains that it
> cannot find a C compiler.
Thanks, I'll do a FreeBSD v11 VM later this week if nobody else gets it
fixed earlier. The python exception you see is due to this one:

def CHECK_STANDARD_LIBPATH(conf):
# at least gcc and clang support this:
try:
cmd = conf.env.CC + ['-print-search-dirs']
out = Utils.cmd_output(cmd).split('\n')
except ValueError:
# option not supported by compiler - use a standard list of directories
dirlist = [ '/usr/lib', '/usr/lib64' ]
except:
raise Errors.WafError('Unexpected error running "%s"' % (cmd))
.

we don't even have conf.env.CC set and get an exception while
defining 'cmd' value, thus it is not defined when the latter except:
statement runs.

I'll look at it.
-- 
/ Alexander Bokovoy

Re: [SCM] Samba Shared Repository - branch master updated

[Amitay Isaacs]

On Wed, Sep 5, 2018 at 6:52 PM, Amitay Isaacs  wrote:
> Hi Alexander,
>
> On Wed, Sep 5, 2018 at 6:41 PM, Alexander Bokovoy  wrote:
>> On ke, 05 syys 2018, Andrew Bartlett wrote:
>>> The branch, master has been updated
>>>
>>> https://git.samba.org/?p=samba.git;a=shortlog;h=master
>> Thank you, Andrew, for the reviews and persistent push to complete this!
>>
>> We are using WAF 2.x now which should allow us to concentrate on
>> switching to Python 3 for the build process as well for all our
>> libraries and components.
>>
>> If you see any build issues related to the WAF, please report them.
>> Autobuild succeeded but we might still have pieces of Python code that
>> aren't excercised through the autobuild targets.
>
> Great job updating to the latest waf.
>
> Now fixing all the corner cases... ;-)
>

Here's another one This time on AIX using xlc...

$ ./configure --enable-developer
Setting top to   :
/home/jenkins/workspace/samba-master/label/ppc64-aix7
Setting out to   :
/home/jenkins/workspace/samba-master/label/ppc64-aix7/ctdb/bin
Checking for 'xlc' (C compiler)  : Traceback (most recent call last):
  File 
"/home/jenkins/workspace/samba-master/label/ppc64-aix7/third_party/waf/waflib/Scripting.py",
line 158, in waf_entry_point
run_commands()
  File 
"/home/jenkins/workspace/samba-master/label/ppc64-aix7/third_party/waf/waflib/Scripting.py",
line 251, in run_commands
ctx = run_command(cmd_name)
  File 
"/home/jenkins/workspace/samba-master/label/ppc64-aix7/third_party/waf/waflib/Scripting.py",
line 235, in run_command
ctx.execute()
  File 
"/home/jenkins/workspace/samba-master/label/ppc64-aix7/third_party/waf/waflib/Configure.py",
line 159, in execute
super(ConfigurationContext, self).execute()
  File 
"/home/jenkins/workspace/samba-master/label/ppc64-aix7/third_party/waf/waflib/Context.py",
line 204, in execute
self.recurse([os.path.dirname(g_module.root_path)])
  File 
"/home/jenkins/workspace/samba-master/label/ppc64-aix7/third_party/waf/waflib/Context.py",
line 286, in recurse
user_function(self)
  File "/home/jenkins/workspace/samba-master/label/ppc64-aix7/ctdb/wscript",
line 118, in configure
conf.RECURSE('lib/replace')
  File "./../buildtools/wafsamba/samba_utils.py", line 30, in fun
return f(*k, **kw)
  File "./../buildtools/wafsamba/samba_utils.py", line 433, in RECURSE
return ctx.recurse(relpath)
  File 
"/home/jenkins/workspace/samba-master/label/ppc64-aix7/third_party/waf/waflib/Context.py",
line 286, in recurse
user_function(self)
  File 
"/home/jenkins/workspace/samba-master/label/ppc64-aix7/third_party/waf/waflib/Utils.py",
line 816, in wrap
ret = fun(*k)
  File 
"/home/jenkins/workspace/samba-master/label/ppc64-aix7/lib/replace/wscript",
line 30, in configure
conf.RECURSE('buildtools/wafsamba')
  File "./../buildtools/wafsamba/samba_utils.py", line 30, in fun
return f(*k, **kw)
  File "./../buildtools/wafsamba/samba_utils.py", line 433, in RECURSE
return ctx.recurse(relpath)
  File 
"/home/jenkins/workspace/samba-master/label/ppc64-aix7/third_party/waf/waflib/Context.py",
line 286, in recurse
user_function(self)
  File 
"/home/jenkins/workspace/samba-master/label/ppc64-aix7/third_party/waf/waflib/Utils.py",
line 816, in wrap
ret = fun(*k)
  File 
"/home/jenkins/workspace/samba-master/label/ppc64-aix7/buildtools/wafsamba/wscript",
line 234, in configure
conf.load('compiler_cc')
  File 
"/home/jenkins/workspace/samba-master/label/ppc64-aix7/third_party/waf/waflib/Configure.py",
line 270, in load
func(self)
  File 
"/home/jenkins/workspace/samba-master/label/ppc64-aix7/third_party/waf/waflib/Tools/compiler_c.py",
line 79, in configure
conf.load(compiler)
  File 
"/home/jenkins/workspace/samba-master/label/ppc64-aix7/third_party/waf/waflib/Configure.py",
line 270, in load
func(self)
  File 
"/home/jenkins/workspace/samba-master/label/ppc64-aix7/third_party/waf/waflib/Tools/xlc.py",
line 59, in configure
conf.find_xlc()
  File "./../buildtools/wafsamba/samba_utils.py", line 30, in fun
return f(*k, **kw)
  File 
"/home/jenkins/workspace/samba-master/label/ppc64-aix7/third_party/waf/waflib/Tools/xlc.py",
line 17, in find_xlc
conf.get_xlc_version(cc)
  File 
"/home/jenkins/workspace/samba-master/label/ppc64-aix7/third_party/waf/waflib/Configure.py",
line 316, in fun
return f(*k, **kw)
  File 
"/home/jenkins/workspace/samba-master/label/ppc64-aix7/third_party/waf/waflib/Tools/c_config.py",
line 1097, in get_xlc_version
cmd = cc + ['-qversion']
TypeError: unsupported operand type(s) for +: 'NoneType' and 'list'


I don't have time to dig deeper this week.  Will spend some time next
week if this issue is not resolved by then.

Amitay.

Re: [SCM] Samba Shared Repository - branch master updated

[Amitay Isaacs]

Hi Alexander,

On Wed, Sep 5, 2018 at 6:41 PM, Alexander Bokovoy  wrote:
> On ke, 05 syys 2018, Andrew Bartlett wrote:
>> The branch, master has been updated
>>via  8de348e third_party: Import exact files from waf-2.0.8/waflib
>>via  8f022a0 script/autobuild: Fix formatting in send_email
>>via  67ed1ea script/autobuild: re-use CACHE_SUFFIX from waflib
>>via  72c3ff9 buildtools/wafsamba: use CACHE_SUFFIX instead of a 
>> hard-coded name
>>via  6fc9f1a lib/mscat: fix logging in wscript
>>via  4c7c10b lib/audit_logging: update to waf 2.0
>>via  f495f64 buildtools/wafsamba: remove ENFORCE_GROUP_ORDERING
>>via  0a9d98b ctdb/wscript: rework how version number is retrieved
>>via  fdd89fe selftest/tests.py: update to support waf 2.0
>>via  72a9e6d wscript: port build_system_heimdal to waf 2.0
>>via  1efe689 wscript: port build_system_mitkrb5 to waf 2.0
>>via  7944ed6 buildtools/wafsamba: port stale_files to waf 2.0
>>via  2a3fcdf buildtools/wafsamba: generate build options output with 
>> waf 2.0
>>via  75d5bcb selftest/wscript: properly handle env.cwd which is a 
>> list, not a string
>>via  31f8945 wafsamba: install Python modules back to bin/python, not 
>> bin/python_modules
>>via  77f3a13 buildtools/wafsamba: use cflags instead of ccflags for 
>> waf 2.0
>>via  1d25ae5 buildtools/wafsamba: use context instead of options for 
>> cross-compile checks for waf 2.0
>>via  aebcd69 buildtools/wafsamba: crosscompile should use 
>> Utils.subprocess in waf 2.0
>>via  fb43723 buildtools/wafsamba: use top and out for waf 2.0
>>via  5c3d31e cdtb/wscript: use top and out for waf 2.0
>>via  699977a wscript: adopt to waf 2.0
>>via  0fdba4b buildtools/wafsamba: use top for waf 2.0
>>via  cdda0d9 buildtools/wafsamba: change SAMBA_BUILD_ENV to use 
>> bldnode.abspath()
>>via  35ebfd3 selftest/wscript: handle lists in environmental 
>> variables in waf
>>via  2a63619 waf heimdal: use absolute path to compile_et
>>via  1fdcbd0 wafsamba: use correct context for APPNAME
>>via  850ceec buildtools/wafsamba: compile asn1 files by adding 
>> missing code from compat15
>>via  2e401d2 wscript: adopt to waf-2.0
>>via  175be93 ctdb/wscript: adopt to waf-2.0
>>via  7aaa1e6 buildtools/wafsamba/samba_abi: always_run helper was 
>> deprecated in waf 2.0
>>via  9f2f5b4 source3/libsmb/wscript: remove unneeded import
>>via  d00ba40 nsswitch/libwbclient/wscript: import from waflib
>>via  944fce2 auth/wscript: import from waflib
>>via  3fc4786 buildtools/wafsamba: add install_dir to build context
>>via  69c655f buildtools/wafsamba: reduce imports
>>via  7eab91a buildtools/wafsamba: adopt to waf 2.0.8
>>via  3b7dfc5 third_party/waf: upgrade to waf 2.0.8
>>via  79c4ba2 auth/wscript: fix options use
>>via  c5ab9ea heimdal wscript changes
>>via  ef4b2d4 wscript_configure_system_mitkrb5: update to handle waf 
>> 2.0.4
>>via  e58ca30 wscript_build_embedded_heimdal: update to handle waf 
>> 2.0.4
>>via  1dc80eb wscript_build: update to handle waf 2.0.4
>>via  c73b779 wscript: update to handle waf 2.0.4
>>via  0119a87 third_party/wscript: update to handle waf 2.0.4
>>via  f14b8cb third_party/uid_wrapper/wscript: update to handle waf 
>> 2.0.4
>>via  051e7e7 third_party/socket_wrapper/wscript: update to handle waf 
>> 2.0.4
>>via  4c44153 third_party/resolv_wrapper/wscript: update to handle waf 
>> 2.0.4
>>via  7e2deed third_party/popt/wscript: update to handle waf 2.0.4
>>via  2847a38 third_party/pam_wrapper/wscript: update to handle waf 
>> 2.0.4
>>via  2b21e67 third_party/nss_wrapper/wscript: update to handle waf 
>> 2.0.4
>>via  3e71f06 third_party/cmocka/wscript: update to handle waf 2.0.4
>>via  ba03d12 third_party/aesni-intel/wscript: update to handle waf 
>> 2.0.4
>>via  ca5008a testsuite/headers/wscript_build: update to handle waf 
>> 2.0.4
>>via  0c423a3 source4/lib/tls/wscript: update to handle waf 2.0.4
>>via  c797e92 source4/heimdal_build/wscript_configure: update to 
>> handle waf 2.0.4
>>via  055aae9 source4/heimdal_build/wscript_build: update to handle 
>> waf 2.0.4
>>via  029ac7d source4/dsdb/samdb/ldb_modules/wscript: update to handle 
>> waf 2.0.4
>>via  8b5ad02 source3/wscript_configure_system_ncurses: update to 
>> handle waf 2.0.4
>>via  91e099d source3/wscript: update to handle waf 2.0.4
>>via  c898f18 source3/build/charset.py: update to handle waf 2.0.4
>>via  0de67cf selftest/wscript: update to handle waf 2.0.4
>>via  fa1ca71 python/wscript: update to handle waf 2.0.4
>>via  7f6ce8f pidl/wscript: update to handle waf 2.0.4
>>via  

Re: [SCM] Samba Shared Repository - branch master updated

[Alexander Bokovoy]

On ke, 05 syys 2018, Andrew Bartlett wrote:
> The branch, master has been updated
>via  8de348e third_party: Import exact files from waf-2.0.8/waflib
>via  8f022a0 script/autobuild: Fix formatting in send_email
>via  67ed1ea script/autobuild: re-use CACHE_SUFFIX from waflib
>via  72c3ff9 buildtools/wafsamba: use CACHE_SUFFIX instead of a 
> hard-coded name
>via  6fc9f1a lib/mscat: fix logging in wscript
>via  4c7c10b lib/audit_logging: update to waf 2.0
>via  f495f64 buildtools/wafsamba: remove ENFORCE_GROUP_ORDERING
>via  0a9d98b ctdb/wscript: rework how version number is retrieved
>via  fdd89fe selftest/tests.py: update to support waf 2.0
>via  72a9e6d wscript: port build_system_heimdal to waf 2.0
>via  1efe689 wscript: port build_system_mitkrb5 to waf 2.0
>via  7944ed6 buildtools/wafsamba: port stale_files to waf 2.0
>via  2a3fcdf buildtools/wafsamba: generate build options output with 
> waf 2.0
>via  75d5bcb selftest/wscript: properly handle env.cwd which is a 
> list, not a string
>via  31f8945 wafsamba: install Python modules back to bin/python, not 
> bin/python_modules
>via  77f3a13 buildtools/wafsamba: use cflags instead of ccflags for 
> waf 2.0
>via  1d25ae5 buildtools/wafsamba: use context instead of options for 
> cross-compile checks for waf 2.0
>via  aebcd69 buildtools/wafsamba: crosscompile should use 
> Utils.subprocess in waf 2.0
>via  fb43723 buildtools/wafsamba: use top and out for waf 2.0
>via  5c3d31e cdtb/wscript: use top and out for waf 2.0
>via  699977a wscript: adopt to waf 2.0
>via  0fdba4b buildtools/wafsamba: use top for waf 2.0
>via  cdda0d9 buildtools/wafsamba: change SAMBA_BUILD_ENV to use 
> bldnode.abspath()
>via  35ebfd3 selftest/wscript: handle lists in environmental variables 
> in waf
>via  2a63619 waf heimdal: use absolute path to compile_et
>via  1fdcbd0 wafsamba: use correct context for APPNAME
>via  850ceec buildtools/wafsamba: compile asn1 files by adding missing 
> code from compat15
>via  2e401d2 wscript: adopt to waf-2.0
>via  175be93 ctdb/wscript: adopt to waf-2.0
>via  7aaa1e6 buildtools/wafsamba/samba_abi: always_run helper was 
> deprecated in waf 2.0
>via  9f2f5b4 source3/libsmb/wscript: remove unneeded import
>via  d00ba40 nsswitch/libwbclient/wscript: import from waflib
>via  944fce2 auth/wscript: import from waflib
>via  3fc4786 buildtools/wafsamba: add install_dir to build context
>via  69c655f buildtools/wafsamba: reduce imports
>via  7eab91a buildtools/wafsamba: adopt to waf 2.0.8
>via  3b7dfc5 third_party/waf: upgrade to waf 2.0.8
>via  79c4ba2 auth/wscript: fix options use
>via  c5ab9ea heimdal wscript changes
>via  ef4b2d4 wscript_configure_system_mitkrb5: update to handle waf 
> 2.0.4
>via  e58ca30 wscript_build_embedded_heimdal: update to handle waf 2.0.4
>via  1dc80eb wscript_build: update to handle waf 2.0.4
>via  c73b779 wscript: update to handle waf 2.0.4
>via  0119a87 third_party/wscript: update to handle waf 2.0.4
>via  f14b8cb third_party/uid_wrapper/wscript: update to handle waf 
> 2.0.4
>via  051e7e7 third_party/socket_wrapper/wscript: update to handle waf 
> 2.0.4
>via  4c44153 third_party/resolv_wrapper/wscript: update to handle waf 
> 2.0.4
>via  7e2deed third_party/popt/wscript: update to handle waf 2.0.4
>via  2847a38 third_party/pam_wrapper/wscript: update to handle waf 
> 2.0.4
>via  2b21e67 third_party/nss_wrapper/wscript: update to handle waf 
> 2.0.4
>via  3e71f06 third_party/cmocka/wscript: update to handle waf 2.0.4
>via  ba03d12 third_party/aesni-intel/wscript: update to handle waf 
> 2.0.4
>via  ca5008a testsuite/headers/wscript_build: update to handle waf 
> 2.0.4
>via  0c423a3 source4/lib/tls/wscript: update to handle waf 2.0.4
>via  c797e92 source4/heimdal_build/wscript_configure: update to handle 
> waf 2.0.4
>via  055aae9 source4/heimdal_build/wscript_build: update to handle waf 
> 2.0.4
>via  029ac7d source4/dsdb/samdb/ldb_modules/wscript: update to handle 
> waf 2.0.4
>via  8b5ad02 source3/wscript_configure_system_ncurses: update to 
> handle waf 2.0.4
>via  91e099d source3/wscript: update to handle waf 2.0.4
>via  c898f18 source3/build/charset.py: update to handle waf 2.0.4
>via  0de67cf selftest/wscript: update to handle waf 2.0.4
>via  fa1ca71 python/wscript: update to handle waf 2.0.4
>via  7f6ce8f pidl/wscript: update to handle waf 2.0.4
>via  e1e5b57 packaging/wscript: update to handle waf 2.0.4
>via  8a74116 nsswitch/wscript_build: update to handle waf 2.0.4
>via  e676183 lib/util/wscript_c

Re: [SCM] Samba Shared Repository - branch master updated

[Andrew Bartlett]

On Sat, 2018-02-24 at 10:56 -0500, jim wrote:
> The wrong version of this patch was pushed.
> The argument to simpleclass_IClassFactory_CreateInstance must be 
> ...**ppv and the value assignment must be  *ppv = (...)ret;
> This was discussed on the list and the corrected patch was created.

Sorry about that.  I'm sure we can fix that up.

Thankfully the code has no callers, the module is part of a subsystem
that is disabled as not even compiling. 

DCOM is an area we explored but never got anywhere, see the thread on
wmic on the samba list a few days ago.

Andrew Bartlett

-- 
Andrew Bartlett   http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT  http://catalyst.net.nz/services/samba

Re: [SCM] Samba Shared Repository - branch master updated

[jim]

The wrong version of this patch was pushed.
The argument to simpleclass_IClassFactory_CreateInstance must be 
...**ppv and the value assignment must be  *ppv = (...)ret;

This was discussed on the list and the corrected patch was created.

On 2/24/2018 9:51 AM, Andrew Bartlett wrote:

-static WERROR simpleclass_IClassFactory_CreateInstance (struct IClassFactory 
*d, TALLOC_CTX *mem_ctx, struct IUnknown *iunk, struct GUID *iid, struct 
IUnknown **ppv)
+static WERROR simpleclass_IClassFactory_CreateInstance(struct IClassFactory *d,
+  TALLOC_CTX *mem_ctx,
+  struct MInterfacePointer 
*pUnknown,
+  struct GUID *iid,
+  struct MInterfacePointer 
*ppv)
  {
struct IStream *ret;
/* FIXME: Check whether IID == ISTREAM_IID */
@@ -71,8 +84,8 @@ static WERROR simpleclass_IClassFactory_CreateInstance 
(struct IClassFactory *d,
ret->vtable = &simple_IStream_vtable;
ret->object_data = NULL;
  
-	*ppv = (struct IUnknown *)ret;

-   
+   ppv = (struct MInterfacePointer *)ret;
+
return WERR_OK;
  }

Re: [SCM] Samba Shared Repository - branch master updated

[Alexander Bokovoy]

On ti, 02 touko 2017, Andreas Schneider wrote:
> The branch, master has been updated
>via  09bc5b5 systemd: fix detection of libsystemd
>   from  2951c59 torture3: Make sure dbwrap_parse_record returns NOT_FOUND 
> for invalid watchers data
> 
> https://git.samba.org/?p=samba.git;a=shortlog;h=master
> 
> 
> - Log -
> commit 09bc5b5374227a555f580c3d7c1d82f15bb818bc
> Author: Alexander Bokovoy 
> Date:   Thu Sep 29 00:09:12 2016 +0300
> 
> systemd: fix detection of libsystemd
> 
> On Fedora 25 detection of libsystemd actually fails due to wrong
> assumptions in the configure test. conf.CHECK_LIB returns a list
> so 'not conf.CHECK_LIB(...)' is always False and we never get to check
> libsystemd.
> 
> Instead, remember result of checking pkg-config for separate
> libsystemd-daemon and libsystemd-journal libraries. If they miss,
> attempt to use libsystemd library instead.
> 
> Signed-off-by: Alexander Bokovoy 
> Reviewed-by: Andreas Schneider 
> 
> Autobuild-User(master): Andreas Schneider 
> Autobuild-Date(master): Tue May  2 13:05:43 CEST 2017 on sn-devel-144
> 
> ---
I created https://bugzilla.samba.org/show_bug.cgi?id=12764 for 4.6
backport.

-- 
/ Alexander Bokovoy

Re: [SCM] Samba Shared Repository - branch master updated

[Alexander Bokovoy]

On pe, 10 maalis 2017, Andrew Bartlett wrote:
> The branch, master has been updated
>via  7142a88 waf: disable-python - don't include python.h in 
> test_headers.c
>via  4455547 autobuild: Add nopython environment to test 
> --disable-python builds (but without tests)
>via  7000fa0 waf: disable-python - don't build torture bits
>via  dbf97e8 waf: disable-python - don't build samba-policy
>via  fa82998 waf: disable-python - don't build samba-net
>via  7eeda99 waf: disable-python - don't build pyrpc_util, dcerpc.py
>via  696d2f7 waf: disable-python - don't build PROVISION, pyparam_util
>via  676e80b waf: disable-python - don't build python/
>via  81ff0f3 waf: disable-python - align tdb's wscript
>via  b45ff0b waf: disable-python - align tevent wscript
>via  ce956be waf: disable-python - align ldb's wscript
>via  dcba0b1 waf: disable-python - align talloc's wscript
>via  7d5db90 waf: disable-python - configuration adjustments
>via  5bbcd09 waf: disable-python - add option globally to build system
>via  9a9e342 waf: disable-python - fix ctdb configuration
>via  8ba2fe5 python: Port the samba.net module to Python 3
>via  0d83cec python: Remove unused import PY3
>via  4ebd877 python: selftest: Add possibility to run old Python test 
> suites with Python 3
>via  64bc64c python: samba.gensec: Port module to Python 3 compatible 
> form
>via  0672fc1 python: samba.gensec: Fix error handling in 
> set_credentials() function
>via  d0914e2 python: selftests: Enable samba.getopt tests execution 
> with Python 3
>via  9ada914 python: samba.getopt: Port module to Python 3 compatible 
> form
>via  da71c39 python: samba.tests.core: Port and enable core tests in 
> Python 3
>via  afe1e83 python: samba.tests: Move import of ported modules out of 
> PY3 condition
>via  e9a4649 python: samba._ldb: Port of samba._ldb to Python 3 
> compatible form
>via  9d8bcea python: samba.tests.auth: Add tests for samba.auth module
>via  1ac5bf2 python: samba.auth: Port samba.auth to Python 3 
> compatible form
>via  47b0148 python: wscript_build: Build some DCE/RPC modules with 
> Python 3
>via  a40d7f1 python: samba.dcerpc: Port security module to Python 3 
> comp. form
>via  e481aed dcerpc/misc tests: asset GUID ordering in python 2 and 3
>via  9843cce python: samba.tests.dcerpc.misc: Port and enable tests
>via  3b1c0ba python: samba.dcerpc: Port RPC related stuff to Python 3
>via  2de5fa4 python: pidl: Port Python interface generator
>via  5edb0b4 python: samba.tests: Enable Python 3 tests for ported 
> modules
>via  b7a5308 buildtools: Work around a . being in the target name when 
> building python3 helpers
>via  fe8bba5 python: wscript_build: Build some modules for Python 3
>via  6fa125e python: Make top-level samba modules Python 3 compatible
>via  211df4a python: samba.tests.dcerpc: Move Class RawDCERPCTest to 
> separated file.
>via  9c55bb9 python: samba.tests.glue: Add new tests for samba._glue.
>via  b454b09 python: samba._glue: Port samba._glue module to Python 3.
>via  5123f15 python: samba.tests.param: Add missing tests
>via  31cef92 python: samba.param: Port param module to Python 3
>via  035e6dc python: samba.tests.credentials: Python 3 compatible tests
>via  1dab2b4 python: samba.credentials: Port pycredentials.c to 
> Python3-compatible form.
>via  583ff0a lib/ldb: Enable use of a python3 pyldb-util system library
>via  f5cafee talloc: use the system pytalloc-util for python3 as well
>via  132cbad scripts/traffic_summary: documentation typo
>via  4485ba5 ./examples/scripts/SambaConfig.py: fix typo in "continue"
>via  d424c7d python/examples/winreg: two variable name typos on a 
> single line
>via  b7cd0f2 python sites/subnets: correctly spell variable name
>via  cedb991 python provision: FDSBackend takes forced uri
>via  b9c5614 python/remove_dc: avoid using non-existent variable
>via  732233f samba-tool domain: correctly spell variable name
>via  495383c python/join: correct spelling of "ctx.del_noerror"
>via  0779d66 selftest: remove unused broken client.py
>via  765275c gitignore: add some hidden files
>via  17dce5d perftests/ad_dc_search: do less work in expensive member 
> searches
>via  bb4ad8d pyldb: p3k readiness: allow single unicode string in msg 
> element
>   from  2772fa4 talloc: fix doxygen of talloc_move
> 
Thanks to everyone for this gigantic work on making Python use in Samba
modern and more supportable going forward.
-- 
/ Alexander Bokovoy

Re: [SCM] Samba Shared Repository - branch master updated

[Jeremy Allison]

On Fri, Dec 02, 2016 at 12:24:03AM +0100, Jeremy Allison wrote:
> The branch, master has been updated
>via  943c6ee s3-libads: Fix canonicalization support with MIT Kerberos
>via  7ca7680 s3: libsmb: Setting the LIBSMBCLIENT_NO_CCACHE 
> environment variable doesn't turn off credential cache use.
>   from  8858657 replace: Include libgen.h if available
> 
> https://git.samba.org/?p=samba.git;a=shortlog;h=master
> 
> 
> - Log -
> commit 943c6ee030a07f87545dcf72e3d1a5bc78eac701
> Author: Andreas Schneider 
> Date:   Mon Feb 15 12:09:59 2016 +0100
> 
> s3-libads: Fix canonicalization support with MIT Kerberos
> 
> This allows to authenticate using user@DOMAIN against an AD DC.
> 
> https://bugzilla.samba.org/show_bug.cgi?id=12457
> 
> Signed-off-by: Andreas Schneider 
> Reviewed-by: Jeremy Allison 
> 
> Autobuild-User(master): Jeremy Allison 
> Autobuild-Date(master): Fri Dec  2 00:23:02 CET 2016 on sn-devel-144
> 
> commit 7ca7680c94f457dea8698124e2780efa55865ed3
> Author: Jeremy Allison 
> Date:   Wed Nov 30 10:25:53 2016 -0800
> 
> s3: libsmb: Setting the LIBSMBCLIENT_NO_CCACHE environment variable 
> doesn't turn off credential cache use.
> 
> Signed-off-by: Jeremy Allison 

Sorry, this was Reviewed-by: Ira but I forgot to add it in.

Re: [SCM] Samba Shared Repository - branch master updated

[Stefan (metze) Metzmacher]

Hi Andrew,

> - Log -
> commit c8c2c850d47a6cdaee5afd84c8ebaa03586c1277
> Author: Andrew Bartlett 
> Date:   Tue Feb 10 12:26:56 2015 +1300
> 
> Update mailing list references to point at lists.samba.org
> 
> The mailing lists are on lists.samba.org, but there are many references 
> that use the shorthand of samba.org
> 
> Some references to samba@ have been changed to samba-technical@ where 
> this make more sense.
> 
> Signed-off-by: Andrew Bartlett 
> Reviewed-by: Garming Sam 
> 
> Autobuild-User(master): Andrew Bartlett 
> Autobuild-Date(master): Tue Feb 10 07:08:28 CET 2015 on sn-devel-104

Can you prepare backports of this for our current release branches?
v4-{0,1,2}-test?

Thanks!
metze



signature.asc
Description: OpenPGP digital signature

Re: [SCM] Samba Shared Repository - branch master updated

[Richard Sharpe]

On Wed, Feb 19, 2014 at 5:13 PM, Jeremy Allison  wrote:
> On Wed, Feb 19, 2014 at 08:20:04PM +0100, Andreas Schneider wrote:
>> The branch, master has been updated
>
> Andreas and David,
>
>>via  b8844fc clitar.c: check all allocations return value
>
> I *HATE* *HATE* *HATE* the idiom used here.
>
> Calling smb_panic on an allocation fail in the
> new tar code is manifestly the *WRONG* thing to
> do.
>
> +/* helper macro to die in case of NULL pointer */
> +#define PANIC_IF_NULL(x) \
> +_panic_if_null(x, __FILE__ ":" STR2(__LINE__) " (" #x ") == NULL\n")
> +
> +/* prototype to silent gcc warning */
> +static inline void* _panic_if_null(void *p, const char *expr);
> +static inline void* _panic_if_null(void *p, const char *expr)
> +{
> +if (!p) {
> +smb_panic(expr);
> +}
> +return p;
> +}
> +
>
> Then later..
>
> +TALLOC_CTX *ctx = PANIC_IF_NULL(talloc_new(NULL));
>
> and:
>
> +fname = PANIC_IF_NULL(talloc_asprintf(ctx,
> +  "%s%s",
> +  client_get_cur_dir(),
> +  buf));
>  if (!fname) {
>  err = 1;
>  goto out;
>
> The above even left the (correct) handling of
> the NULL return alone !
>
> Remember on common setups this will call a
> panic action script which will leave the
> client hanging on a sleep 999 call and
> leave the caller with a hung command and no
> indication of what failed.
>
> Can you fix this please to do proper error
> checking of a NULL return and to bail out
> of the tar command with a correct error indication
> that the command failed ?
>
> This really needs fixing properly before this code
> can go anywhere near a production branch.

Personally, I would like to lobby for an extra parameter to smb_panic,
called dump_core or something.

Here is a situation where we don't need to dump core:

source3/smbd/server.c#6 (text)

@@ -640,6 +640,18 @@
 "because too many files are open\n"));
goto exit;
}
+
+   /*
+* If we get NT_STATUS_OPEN_FAILED, there was
a problem with
+* re-initing the TDBs, so complain, but do
not dump core
+* as this can exacerbate the problems.
+*/
+   if (NT_STATUS_EQUAL(status, NT_STATUS_OPEN_FAILED)) {
+   DEBUG(0, ("FATAL: child process cannot
initialize "
+ "because one or more TDBs
could not be reopened\n"));
+   goto exit;
+   }
+
if (lp_clustering() &&
NT_STATUS_EQUAL(status,
NT_STATUS_INTERNAL_DB_ERROR)) {

This happens after reinit_after_fork ...

When it happens because tdb_reopen_all failed, we should probably not
dump core, especially as that only exacerbates existing problems and
the core file tells us absolutely nothing new (the error in
tdb_reopen_all is long gone.)

-- 
Regards,
Richard Sharpe
(何以解憂？唯有杜康。--曹操)

Re: [SCM] Samba Shared Repository - branch master updated

[Volker Lendecke]

On Thu, Jun 12, 2014 at 04:02:04PM +0200, David Disseldorp wrote:
> The branch, master has been updated
>via  0057388 leases: Fix uint32->hyper
>via  2f15208 Remove .clang_complete.
>   from  0b4af49 waf: fix the name of the WINBIND "nss" module on AIX
> 
> http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master
> 
> 
> - Log -
> commit 00573880e5f0362c3c442e82501bb0efe731ce7b
> Author: Volker Lendecke 
> Date:   Thu Jun 12 08:53:36 2014 +
> 
> leases: Fix uint32->hyper
> 
> Thanks to Noel Power  for finding this!
> 
> Signed-off-by: Volker Lendecke 
> Reviewed-by: Noel Power 
> [dd...@samba.org: changed Noel's address, as requested]
> Reviewed-by: David Disseldorp 

Well, if I take a look at

https://lists.samba.org/archive/samba-technical/2014-June/100408.html

I see a "Noel Power nopower at suse.com" which to me is a
pretty strong indication that Noel's address is indeed
nopo...@suse.com. Where is the problem with that?

Thanks,

Volker

-- 
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-37-0, fax: +49-551-37-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de, mailto:kont...@sernet.de

Re: [SCM] Samba Shared Repository - branch master updated

[David Disseldorp]

Hi Andreas,

On Thu,  5 Jun 2014 13:19:04 +0200 (CEST), Andreas Schneider wrote:

> Author: Andreas Schneider 
> Date:   Wed Apr 2 09:01:41 2014 +0200
> 
> Add YouCompleteMe config.

Would have been good to see this go by the list beforehand...

I'm not a fan of seeing developer environment tools, editor modelines,
add-ons and other accessories go into the source repo. IMO this kind of
cruft should remain on the developers' system - people have differing
setups.

Cheers, David

Re: [SCM] Samba Shared Repository - branch master updated

[Jeremy Allison]

On Wed, Feb 19, 2014 at 08:20:04PM +0100, Andreas Schneider wrote:
> The branch, master has been updated

Andreas and David,

>via  b8844fc clitar.c: check all allocations return value

I *HATE* *HATE* *HATE* the idiom used here.

Calling smb_panic on an allocation fail in the
new tar code is manifestly the *WRONG* thing to
do.

+/* helper macro to die in case of NULL pointer */
+#define PANIC_IF_NULL(x) \
+_panic_if_null(x, __FILE__ ":" STR2(__LINE__) " (" #x ") == NULL\n")
+
+/* prototype to silent gcc warning */
+static inline void* _panic_if_null(void *p, const char *expr);
+static inline void* _panic_if_null(void *p, const char *expr)
+{
+if (!p) {
+smb_panic(expr);
+}
+return p;
+}
+

Then later..

+TALLOC_CTX *ctx = PANIC_IF_NULL(talloc_new(NULL));

and:

+fname = PANIC_IF_NULL(talloc_asprintf(ctx,
+  "%s%s",
+  client_get_cur_dir(),
+  buf));
 if (!fname) {
 err = 1;
 goto out;

The above even left the (correct) handling of
the NULL return alone !

Remember on common setups this will call a
panic action script which will leave the
client hanging on a sleep 999 call and
leave the caller with a hung command and no
indication of what failed.

Can you fix this please to do proper error
checking of a NULL return and to bail out
of the tar command with a correct error indication
that the command failed ?

This really needs fixing properly before this code
can go anywhere near a production branch.

Jeremy.

Re: [SCM] Samba Shared Repository - branch master updated

[Jelmer Vernooij]

On Thu, Jan 23, 2014 at 11:52:04PM +0100, Andrew Bartlett wrote:
> The branch, master has been updated
>via  6104b1f samba-tool classicupgrade: Remove unsued upgrade_smbconf
>via  6c6c3fa samba-tool classicupgrade: Remove unsued reference to 
> samba3sam
>   from  7638f52 s3: Don't open the dir for stat/attr/acl related mask
> 
> http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master
> 
> 
> - Log -
> commit 6104b1fe98798f275d0fe5f81a678941ab29b184
> Author: Andrew Bartlett 
> Date:   Thu Jan 16 15:08:16 2014 +1300
> 
> samba-tool classicupgrade: Remove unsued upgrade_smbconf
> 
> Signed-off-by: Andrew Bartlett 
> Reviewed-By: Jelmer Vernooij 
> 
> Autobuild-User(master): Andrew Bartlett 
> Autobuild-Date(master): Thu Jan 23 23:51:56 CET 2014 on sn-devel-104
> 
> commit 6c6c3fa7ccb1415addd9553c68fc21115b7c23da
> Author: Andrew Bartlett 
> Date:   Thu Jan 16 15:07:31 2014 +1300
> 
> samba-tool classicupgrade: Remove unsued reference to samba3sam
> 
> Signed-off-by: Andrew Bartlett 
> Reviewed-By: Jelmer Vernooij 

gwenhwyvar:~/src/samba% git log  | grep -i unsued | wc -l
7

:-)


signature.asc
Description: Digital signature

Re: [SCM] Samba Shared Repository - branch master updated

[Jelmer Vernooij]

On Wed, Jan 08, 2014 at 08:13:04AM +0100, Andrew Bartlett wrote:
> The branch, master has been updated
>via  f8363dd crypto: fix build on OS X
>via  af5102f build: test the generic md5 function after importing it 
> from hashlib
>   from  8e5f4ea libgpo: apply some const.
> 
> http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master
> 
> 
> - Log -
> commit f8363dd22a67108022bc0fe95003450f79057f7d
> Author: Björn Jacke 
> Date:   Tue Jan 7 15:55:57 2014 +0100
> 
> crypto: fix build on OS X
> 
> we also need to use the CC_MD5_CTX from CommonCrypto here instead of the 
> MD5_CTX
> 
> Signed-off-by: Bjoern Jacke 
> 
> Reviewed-by: Andrew Bartlett 
> 
> Autobuild-User(master): Andrew Bartlett 
> Autobuild-Date(master): Wed Jan  8 08:12:29 CET 2014 on sn-devel-104
> 
> commit af5102fe0075520d62beba412c994d4a74f5446b
> Author: Björn Jacke 
> Date:   Tue Jan 7 15:55:56 2014 +0100
> 
> build: test the generic md5 function after importing it from hashlib
> 
> otherwise we used the one from md5 which lead to the following warning on 
> SerNet-imini:
> 
> the md5 module is deprecated; use hashlib instead import md5

Do we still support Python 2.4 ? It does not have hashlib, just the md5 module.

Cheers,

jelmer

Re: [SCM] Samba Shared Repository - branch master updated

[Simo]

On Mon, 2013-12-16 at 13:05 -0800, Jeremy Allison wrote:
> On Mon, Dec 16, 2013 at 03:56:16PM -0500, Simo wrote:
> > On Mon, 2013-12-16 at 10:34 -0800, Jeremy Allison wrote:
> > > On Sat, Dec 14, 2013 at 01:20:04PM +0100, Stefan Metzmacher wrote:
> > > > 
> > > > commit 5baa7402ba8eebd9b2ddc6b259ae9bb2852b4bb1
> > > > Author: Volker Lendecke 
> > > > Date:   Fri Dec 6 14:34:05 2013 +
> > > > 
> > > > smbd: Implement and use full_path_tos
> > > > 
> > > > Yes, this looks like a hack, but talloc_asprintf does show up high 
> > > > in
> > > > profiles called from these routines
> > > > 
> > > > Signed-off-by: Volker Lendecke 
> > > > Reviewed-by: Stefan Metzmacher 
> > > 
> > > Hahaha ! Coming this summer to a cinema near you :
> > > 
> > > "Samba 4 - Revenge of the pstrings" :-) :-).
> > 
> > Uhm, given this string is never passed anywhere and you want to use the
> > stack, why didn't you simply use alloca() + snprintf() ?
> > 
> > That would seem a lot simpler and as effective to me, am I missing
> > something ?
> 
> I don't think alloca is as portable as the code
> Volker wrote.

I see.

Simo.

Re: [SCM] Samba Shared Repository - branch master updated

[Jeremy Allison]

On Mon, Dec 16, 2013 at 03:56:16PM -0500, Simo wrote:
> On Mon, 2013-12-16 at 10:34 -0800, Jeremy Allison wrote:
> > On Sat, Dec 14, 2013 at 01:20:04PM +0100, Stefan Metzmacher wrote:
> > > 
> > > commit 5baa7402ba8eebd9b2ddc6b259ae9bb2852b4bb1
> > > Author: Volker Lendecke 
> > > Date:   Fri Dec 6 14:34:05 2013 +
> > > 
> > > smbd: Implement and use full_path_tos
> > > 
> > > Yes, this looks like a hack, but talloc_asprintf does show up high in
> > > profiles called from these routines
> > > 
> > > Signed-off-by: Volker Lendecke 
> > > Reviewed-by: Stefan Metzmacher 
> > 
> > Hahaha ! Coming this summer to a cinema near you :
> > 
> > "Samba 4 - Revenge of the pstrings" :-) :-).
> 
> Uhm, given this string is never passed anywhere and you want to use the
> stack, why didn't you simply use alloca() + snprintf() ?
> 
> That would seem a lot simpler and as effective to me, am I missing
> something ?

I don't think alloca is as portable as the code
Volker wrote.

Re: [SCM] Samba Shared Repository - branch master updated

[Simo]

On Mon, 2013-12-16 at 10:34 -0800, Jeremy Allison wrote:
> On Sat, Dec 14, 2013 at 01:20:04PM +0100, Stefan Metzmacher wrote:
> > 
> > commit 5baa7402ba8eebd9b2ddc6b259ae9bb2852b4bb1
> > Author: Volker Lendecke 
> > Date:   Fri Dec 6 14:34:05 2013 +
> > 
> > smbd: Implement and use full_path_tos
> > 
> > Yes, this looks like a hack, but talloc_asprintf does show up high in
> > profiles called from these routines
> > 
> > Signed-off-by: Volker Lendecke 
> > Reviewed-by: Stefan Metzmacher 
> 
> Hahaha ! Coming this summer to a cinema near you :
> 
> "Samba 4 - Revenge of the pstrings" :-) :-).

Uhm, given this string is never passed anywhere and you want to use the
stack, why didn't you simply use alloca() + snprintf() ?

That would seem a lot simpler and as effective to me, am I missing
something ?

Simo.

Re: [SCM] Samba Shared Repository - branch master updated

[Jeremy Allison]

On Sat, Dec 14, 2013 at 01:20:04PM +0100, Stefan Metzmacher wrote:
> 
> commit 5baa7402ba8eebd9b2ddc6b259ae9bb2852b4bb1
> Author: Volker Lendecke 
> Date:   Fri Dec 6 14:34:05 2013 +
> 
> smbd: Implement and use full_path_tos
> 
> Yes, this looks like a hack, but talloc_asprintf does show up high in
> profiles called from these routines
> 
> Signed-off-by: Volker Lendecke 
> Reviewed-by: Stefan Metzmacher 

Hahaha ! Coming this summer to a cinema near you :

"Samba 4 - Revenge of the pstrings" :-) :-).

Great work though. I take it this was a cachegrind
find ?

Cheers,

Jeremy.

Re: [SCM] Samba Shared Repository - branch master updated

[Stefan (metze) Metzmacher]

> commit 12a2230581b3ff5c7a29819532652d7ddfe61521
> Author: Andreas Schneider 
> Date:   Fri Nov 8 16:14:35 2013 +0100
> 
> s4-smb_server: Fix a use after free.
> 
> If we haven't allocated the smbsrv_session then we should not free it.
> 
> Signed-off-by: Andreas Schneider 
> Reviewed-by: Jeremy Allison 
>
> diff --git a/source4/smb_server/smb/sesssetup.c 
> b/source4/smb_server/smb/sesssetup.c
> index b26c128..4ebc0c4 100644
> --- a/source4/smb_server/smb/sesssetup.c
> +++ b/source4/smb_server/smb/sesssetup.c
> @@ -415,6 +415,7 @@ static void sesssetup_spnego(struct smbsrv_request *req, 
> union smb_sesssetup *se
>  {
>   NTSTATUS status;
>   struct smbsrv_session *smb_sess = NULL;
> + bool is_smb_sess_new = false;
>   struct sesssetup_spnego_state *s = NULL;
>   uint16_t vuid;
>   struct tevent_req *subreq;
> @@ -465,6 +466,7 @@ static void sesssetup_spnego(struct smbsrv_request *req, 
> union smb_sesssetup *se
>   status = NT_STATUS_INSUFFICIENT_RESOURCES;
>   goto failed;
>   }
> + is_smb_sess_new = true;
>   } else {
>   smb_sess = smbsrv_session_find_sesssetup(req->smb_conn, vuid);
>   }
> @@ -510,7 +512,9 @@ static void sesssetup_spnego(struct smbsrv_request *req, 
> union smb_sesssetup *se
>  nomem:
>   status = NT_STATUS_NO_MEMORY;
>  failed:
> - talloc_free(smb_sess);
> + if (is_smb_sess_new) {
> + talloc_free(smb_sess);
> + }
>   status = nt_status_squash(status);
>   smbsrv_sesssetup_backend_send(req, sess, status);

I think we need to talloc_steal(req, smb_sess) here.
This is similar to
https://git.samba.org/?p=samba.git;a=commitdiff;h=25494628a2e977568de0f634602ebe893d0a5b88

metze

Re: [SCM] Samba Shared Repository - branch master updated

[Michael Adam]

On 2013-09-11 at 10:18 +0200, Volker Lendecke wrote:
> On Wed, Sep 11, 2013 at 10:16:02AM +0200, Volker Lendecke wrote:
> 
> Gna. Pushed from the wrong tree. This was meant to carry
> Reviewed-by: Jeremy Allison 
> tags. Push a revert?

I'd say there is no point. It is in the tree and you would
have to get review for the revert first... ;) The important
point is that review has in fact happened.

This would not have happened, if we had installed a
mechanism that would reject a push if one at least of
the commits does not contain at least two signoff/review
tags by team members... (Hint, hint... ;-)

Cheers - Michael


signature.asc
Description: Digital signature

Re: [SCM] Samba Shared Repository - branch master updated

[Jeremy Allison]

On Wed, Sep 11, 2013 at 10:18:50AM +0200, Volker Lendecke wrote:
> On Wed, Sep 11, 2013 at 10:16:02AM +0200, Volker Lendecke wrote:
> > The branch, master has been updated
> >via  73278cd smbd: Convert br_lck->lock_data to talloc
> >via  20cc710 smbd: Move "struct byte_range_lock" definition to 
> > brlock.c
> >via  07948ef smbd: Add brl_fsp access function
> >via  6bcfc1a smbd: Add brl_num_locks access function
> >via  635c35d smbd: Use ZERO_STRUCT instead of memset
> >via  e2bb3b0 smbd: Fix a typo
> >via  2aa6844 smbd: Make brl_lock_failed static
> >via  cca8faa smbd: Make brl_same_context static
> >via  e533bf3 smbd: Fix blank line endings
> >   from  d809cf6 Raise the level of a debug.
> > 
> > http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master
> > 
> > 
> > - Log -
> > commit 73278cd19bc80854a2b23ef787c3ebdc3fa56bfd
> > Author: Volker Lendecke 
> > Date:   Tue Sep 10 21:04:47 2013 +0200
> > 
> > smbd: Convert br_lck->lock_data to talloc
> > 
> > Autobuild-User(master): Volker Lendecke 
> > Autobuild-Date(master): Wed Sep 11 10:15:38 CEST 2013 on sn-devel-104
> 
> Gna. Pushed from the wrong tree. This was meant to carry
> Reviewed-by: Jeremy Allison 
> tags. Push a revert?

No, not worth it IMHO. These messages are archived so anyone
can check this was correctly reviewed.

Jeremy.

Re: [SCM] Samba Shared Repository - branch master updated

[Volker Lendecke]

On Wed, Sep 11, 2013 at 10:16:02AM +0200, Volker Lendecke wrote:
> The branch, master has been updated
>via  73278cd smbd: Convert br_lck->lock_data to talloc
>via  20cc710 smbd: Move "struct byte_range_lock" definition to brlock.c
>via  07948ef smbd: Add brl_fsp access function
>via  6bcfc1a smbd: Add brl_num_locks access function
>via  635c35d smbd: Use ZERO_STRUCT instead of memset
>via  e2bb3b0 smbd: Fix a typo
>via  2aa6844 smbd: Make brl_lock_failed static
>via  cca8faa smbd: Make brl_same_context static
>via  e533bf3 smbd: Fix blank line endings
>   from  d809cf6 Raise the level of a debug.
> 
> http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master
> 
> 
> - Log -
> commit 73278cd19bc80854a2b23ef787c3ebdc3fa56bfd
> Author: Volker Lendecke 
> Date:   Tue Sep 10 21:04:47 2013 +0200
> 
> smbd: Convert br_lck->lock_data to talloc
> 
> Autobuild-User(master): Volker Lendecke 
> Autobuild-Date(master): Wed Sep 11 10:15:38 CEST 2013 on sn-devel-104

Gna. Pushed from the wrong tree. This was meant to carry
Reviewed-by: Jeremy Allison 
tags. Push a revert?

Sorry!

Volker

-- 
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-37-0, fax: +49-551-37-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de, mailto:kont...@sernet.de

*
visit us on it-sa:IT security exhibitions in Nürnberg, Germany
October 8th - 10th 2013, hall 12, booth 333
free tickets available via code 270691 on: www.it-sa.de/gutschein
**

Re: [SCM] Samba Shared Repository - branch master updated

[Andreas Schneider]

On Friday 08 February 2013 17:06:12 Jeremy Allison wrote:
> On Thu, Feb 07, 2013 at 03:51:05PM +0100, Andreas Schneider wrote:
> > The branch, master has been updated
> > 
> >via  62e6ea4 s3-rpc_server: Fix password encoding in
> >_netr_ServerGetTrustInfo(). via  823a25f s3-rpc_server: Dont wipe
> >out ref pointers in _netr_ServerGetTrustInfo().>   
> >   from  ff496ef autoconf: rename pdb_ldap module to pdb_ldapsam
> > 
> > commit 62e6ea408a20197420eadb133df6ba233c7874b4
> > Author: Sumit Bose 
> > Date:   Mon Feb 4 13:15:18 2013 +0100
> > 
> > s3-rpc_server: Fix password encoding in _netr_ServerGetTrustInfo().
> > 
> > Reviewed-by: Günther Deschner 
> > Reviewed-by: Andreas Schneider 
> > 
> > commit 823a25f6a47c311ab33874be8a889ffbaf8db527
> > Author: Günther Deschner 
> > Date:   Tue Dec 11 09:29:37 2012 +0100
> > 
> > s3-rpc_server: Dont wipe out ref pointers in
> > _netr_ServerGetTrustInfo().
> > 
> > Guenther
> > 
> > Signed-off-by: Günther Deschner 
> > Reviewed-by: Andreas Schneider 
> 
> Do we need these in 3.6.x and 4.0.x ?
> 
> Jeremy.

I think so, I thought Sumit or Günther created a bug for it.

Günther?


-- andreas

-- 
Andreas Schneider   GPG-ID: F33E3FC6
Samba Team a...@samba.org
www.samba.org

Re: [SCM] Samba Shared Repository - branch master updated

[Jeremy Allison]

On Thu, Feb 07, 2013 at 03:51:05PM +0100, Andreas Schneider wrote:
> The branch, master has been updated

>via  62e6ea4 s3-rpc_server: Fix password encoding in 
> _netr_ServerGetTrustInfo().
>via  823a25f s3-rpc_server: Dont wipe out ref pointers in 
> _netr_ServerGetTrustInfo().
>   from  ff496ef autoconf: rename pdb_ldap module to pdb_ldapsam

> commit 62e6ea408a20197420eadb133df6ba233c7874b4
> Author: Sumit Bose 
> Date:   Mon Feb 4 13:15:18 2013 +0100
> 
> s3-rpc_server: Fix password encoding in _netr_ServerGetTrustInfo().
> 
> Reviewed-by: Günther Deschner 
> Reviewed-by: Andreas Schneider 
> 
> commit 823a25f6a47c311ab33874be8a889ffbaf8db527
> Author: Günther Deschner 
> Date:   Tue Dec 11 09:29:37 2012 +0100
> 
> s3-rpc_server: Dont wipe out ref pointers in _netr_ServerGetTrustInfo().
> 
> Guenther
> 
> Signed-off-by: Günther Deschner 
> Reviewed-by: Andreas Schneider 

Do we need these in 3.6.x and 4.0.x ?

Jeremy.

Re: [SCM] Samba Shared Repository - branch master updated

[Jelmer Vernooij]

On Sun, Nov 11, 2012 at 03:49:02PM +0100, Andrew Bartlett wrote:
> The branch, master has been updated
>via  1d81e52 selftest: Add tests for expected behaviour on directories 
> as well as files
>via  a6a0155 pysmbd: Add SMB_ACL_EXECUTE to the mask set by 
> make_simple_acl()
>via  312f8dd selftest: Make samba.tests.ntacl also use 
> TestCaseInTempDir
>via  b4d8629 samba-tool: Rework ldap attribute fetch in classicupgrade 
> for missing attributes
>   from  c06d602 s3:smbd: Fix typo in got_duplicate_group check
> 
> http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master
> 
> 
> - Log -
> commit 1d81e52bba65f05378db7027537aa27eb5bfa70a
> Author: Andrew Bartlett 
> Date:   Sun Nov 11 21:33:41 2012 +1100
> 
> selftest: Add tests for expected behaviour on directories as well as files
> 
> This is important because it covers the codepath which had the talloc
> error fixed by commit 60cf4cb5a630506747431ecbf00d890509baf2f3
> (vfs_acl_common: In add_directory_inheritable_components allocate on
> psd as parent)
> 
> Andrew Bartlett
> 
> Signed-off-by: Andrew Bartlett 
> Reviewed-by: Jelmer Vernooij 
> Reviewed-by: Andrew Bartlett 
You've reviewed your own changes ? I'm shocked. :-) Is this a
script gone crazy perhaps ?

Cheers,

Jelmer

Re: [SCM] Samba Shared Repository - branch master updated

[simo]

On Tue, 2012-10-30 at 23:57 +0100, Andrew Bartlett wrote:
> commit cc6d0decc7980028293168aee267e7610752fc80
> Author: Andrew Bartlett 
> Date:   Tue Oct 30 10:21:42 2012 +1100
> 
> ldb: Change ltdb_unpack_data to take an ldb_context
> 
> It always de-references the module to find the ldb anyway.
> 
> Andrew Bartlett 

Andrew,
why are you messing over with these interface conventions ?

I see no rationale for this change, can you please revert and learn a
bit about consistent and predictable interfaces ?

Thanks.

Simo.

-- 
Simo Sorce
Samba Team GPL Compliance Officer 
Principal Software Engineer at Red Hat, Inc. 

Re: [SCM] Samba Shared Repository - branch master updated

[Michael Adam]

Hi Jeremy,

On 2012-10-20 at 00:57 -0700, Jeremy Allison wrote:
> On Fri, Oct 19, 2012 at 09:03:46PM +0200, Michael Adam wrote:
> > On 2012-10-19 at 10:14 -0700, Jeremy Allison wrote:
> > > Can you guys split these checks out into a static function
> > > that you call once from vfs_default_durable_reconnect() ?
> > > 
> > > Putting these checks inside that function makes it a million
> > > lines long and hard to read :-).
> > 
> > /me goes counting lines and major culprits in various even longer
> > functions like open_file_ntcreate()... ;-)
> 
> Yes I know. And I got beaten up so much for that I never
> did it again :-).

:)

> And vl and I have been slimming it down ever since :-).

True, and that is very helpful, indeed.

> > Seriously: Certainly, we can do that.
> > Do you mean a single function of each of the checks, or would
> > you be fine with one function for all the stat detail checks?
> 
> One function for all of the stat checks works for me. That
> function just goes through a series of logical steps, to
> me it makes sense for it to be factored out into independent
> logical groups (i.e. the stat checks are one) that are put
> into separate static functions.

It definitley does make sense. I will factor it out
and run the patch via you. Thanks for the hint.

Cheers - Michael



pgpQ8wf9yXElb.pgp
Description: PGP signature

Re: [SCM] Samba Shared Repository - branch master updated

[Jeremy Allison]

On Fri, Oct 19, 2012 at 09:03:46PM +0200, Michael Adam wrote:
> On 2012-10-19 at 10:14 -0700, Jeremy Allison wrote:
> > Can you guys split these checks out into a static function
> > that you call once from vfs_default_durable_reconnect() ?
> > 
> > Putting these checks inside that function makes it a million
> > lines long and hard to read :-).
> 
> /me goes counting lines and major culprits in various even longer
> functions like open_file_ntcreate()... ;-)

Yes I know. And I got beaten up so much for that I never
did it again :-). And vl and I have been slimming it down
ever since :-).

> Seriously: Certainly, we can do that.
> Do you mean a single function of each of the checks, or would
> you be fine with one function for all the stat detail checks?

One function for all of the stat checks works for me. That
function just goes through a series of logical steps, to
me it makes sense for it to be factored out into independent
logical groups (i.e. the stat checks are one) that are put
into separate static functions.

Thanks !

Jeremy.

Re: [SCM] Samba Shared Repository - branch master updated

[Michael Adam]

On 2012-10-19 at 10:14 -0700, Jeremy Allison wrote:
> On Fri, Oct 19, 2012 at 06:57:03PM +0200, Stefan Metzmacher wrote:
> > The branch, master has been updated
> >via  7d05ff7 s3:smbd:durable: store stat info for disconnected 
> > durables and check at reconnect.
> >via  a6726f9 s3:open_files.idl: add stat-info to 
> > vfs_default_durable_cookie.
> >via  660e90d s3:smbd/durable: remove an extra blank line from 
> > vfs_default_durable_reconnect()
> >   from  24d225f s3:smbd/durable: add write_time specific stuff to 
> > vfs_default_durable_cookie
> > 
> > http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master
> > 
> > 
> > - Log -
> > commit 7d05ff77264cf48d1ee82ca19c8c7e6951e64cf5
> > Author: Michael Adam 
> > Date:   Fri Oct 19 16:00:05 2012 +0200
> > 
> > s3:smbd:durable: store stat info for disconnected durables and check at 
> > reconnect.
> > 
> > At durable reconnect, we check the stat info stored in the cookie 
> > against the
> > current stat data from the file we just opened.  If any detail differs, 
> > we deny
> > the durable reconnect, because in that case it is very likely that 
> > someone
> > opened the file while the handle was disconnected, which has to be 
> > interpreted
> > as an oplock break.
> 
> Can you guys split these checks out into a static function
> that you call once from vfs_default_durable_reconnect() ?
> 
> Putting these checks inside that function makes it a million
> lines long and hard to read :-).

/me goes counting lines and major culprits in various even longer
functions like open_file_ntcreate()... ;-)

Seriously: Certainly, we can do that.
Do you mean a single function of each of the checks, or would
you be fine with one function for all the stat detail checks?

Cheers - Michael



pgpUPLlVGDpLn.pgp
Description: PGP signature

Re: [SCM] Samba Shared Repository - branch master updated

[Jeremy Allison]

On Fri, Oct 19, 2012 at 06:57:03PM +0200, Stefan Metzmacher wrote:
> The branch, master has been updated
>via  7d05ff7 s3:smbd:durable: store stat info for disconnected 
> durables and check at reconnect.
>via  a6726f9 s3:open_files.idl: add stat-info to 
> vfs_default_durable_cookie.
>via  660e90d s3:smbd/durable: remove an extra blank line from 
> vfs_default_durable_reconnect()
>   from  24d225f s3:smbd/durable: add write_time specific stuff to 
> vfs_default_durable_cookie
> 
> http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master
> 
> 
> - Log -
> commit 7d05ff77264cf48d1ee82ca19c8c7e6951e64cf5
> Author: Michael Adam 
> Date:   Fri Oct 19 16:00:05 2012 +0200
> 
> s3:smbd:durable: store stat info for disconnected durables and check at 
> reconnect.
> 
> At durable reconnect, we check the stat info stored in the cookie against 
> the
> current stat data from the file we just opened.  If any detail differs, 
> we deny
> the durable reconnect, because in that case it is very likely that someone
> opened the file while the handle was disconnected, which has to be 
> interpreted
> as an oplock break.

Can you guys split these checks out into a static function
that you call once from vfs_default_durable_reconnect() ?

Putting these checks inside that function makes it a million
lines long and hard to read :-).

Thanks,

Jeremy.

Re: [SCM] Samba Shared Repository - branch master updated

[Kai Blin]

On 2012-10-11 21:23, Stefan (metze) Metzmacher wrote:
> Hi Jelmer,
> 
>> - Log -
>> commit c2d14747d608d406de6410556807d467cd0b85ef
>> Author: Jelmer Vernooij 
>> Date:   Thu Oct 11 14:45:10 2012 +0200
>>
>> provision: Always create DNS user.
>> 
>> The DNS user is currently only used by the bind9 plugin. This makes it
>> easier to later on switch between the builtin DNS server and bind
>> backend.
>> 
>> In addition, ideally the internal DNS server would use that (separate)
>> user too.
> 
> Why? Isn't that the job of samba_upgradedns?
> I removed this behavior because I want us to match windows as much as
> possible.

+1

Cheers,
Kai

-- 
Kai Blin
Worldforge developer http://www.worldforge.org/
Wine developer http://wiki.winehq.org/KaiBlin
Samba team member http://www.samba.org/samba/team/

Re: [SCM] Samba Shared Repository - branch master updated

[Stefan (metze) Metzmacher]

Hi Jelmer,

> - Log -
> commit c2d14747d608d406de6410556807d467cd0b85ef
> Author: Jelmer Vernooij 
> Date:   Thu Oct 11 14:45:10 2012 +0200
> 
> provision: Always create DNS user.
> 
> The DNS user is currently only used by the bind9 plugin. This makes it
> easier to later on switch between the builtin DNS server and bind
> backend.
> 
> In addition, ideally the internal DNS server would use that (separate)
> user too.

Why? Isn't that the job of samba_upgradedns?
I removed this behavior because I want us to match windows as much as
possible.

metze



signature.asc
Description: OpenPGP digital signature

Re: [SCM] Samba Shared Repository - branch master updated

[Volker Lendecke]

On Wed, Aug 29, 2012 at 04:06:02PM +0200, Björn Jacke wrote:
> The branch, master has been updated
>via  f31d0d0 vfs_media_harmony: fix some compile warnings with llvm
>   from  fb15e5a s3-printing: fix bug 9123 lprng job tracking errors
> 
> http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master
> 
> 
> - Log -
> commit f31d0d0e30af6577de483ec4e811b9422b8ef39a
> Author: Björn Jacke 
> Date:   Wed Aug 29 13:37:05 2012 +0200
> 
> vfs_media_harmony: fix some compile warnings with llvm
> 
> Autobuild-User(master): Björn Jacke 
> Autobuild-Date(master): Wed Aug 29 16:05:10 CEST 2012 on sn-devel-104
> 
> ---
> 
> Summary of changes:
>  source3/modules/vfs_media_harmony.c |  132 +-
>  1 files changed, 66 insertions(+), 66 deletions(-)
> 
> 
> Changeset truncated at 500 lines:
> 
> diff --git a/source3/modules/vfs_media_harmony.c 
> b/source3/modules/vfs_media_harmony.c
> index e2cb407..53cb176 100644
> --- a/source3/modules/vfs_media_harmony.c
> +++ b/source3/modules/vfs_media_harmony.c
> @@ -430,7 +430,7 @@ static int alloc_get_client_path(vfs_handle_struct 
> *handle,
>   (*newPath)[pathPtr - path + CREATING_DIRNAME_LEN] = '\0';
>   DEBUG(MH_INFO_DEBUG, ("newPath #2 %s\n", *newPath));
>  
> - if (status = alloc_append_client_suffix(handle, newPath))
> + if ((status = alloc_append_client_suffix(handle, newPath)))

Can you make that into two lines? This form will create
warnings on other compilers.

status = ...
if (status != NULL) {
}

is the one to use according to README.Coding.

Thanks,

Volker

-- 
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-37-0, fax: +49-551-37-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de, mailto:kont...@sernet.de

Re: [SCM] Samba Shared Repository - branch master updated

[simo]

On Thu, 2012-08-09 at 08:21 +0200, Andrew Bartlett wrote: 
> The branch, master has been updated
>via  a117fd6 s4-dsdb: Ensure we have indexing enabled during the 
> provision
>via  ef87b4e s4-pydsdb: Provide control of if we should write index 
> attributes when reloading a schema
>   from  1a1f01e s4-dsdb: Change talloc parent
> 
> http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master
> 
> 
> - Log -
> commit a117fd6d11aef5360a1af1fc6da542829da8b7b0
> Author: Andrew Bartlett 
> Date:   Thu Aug 9 14:33:49 2012 +1000
> 
> s4-dsdb: Ensure we have indexing enabled during the provision
> 
> Because we set the schema before we connected the ldb to a file, the 
> @INDEX records
> were not added until next startup.  This cost 100% more time in running 
> provision on
> my laptop.
> 
> Andrew Bartlett
> 
> Autobuild-User(master): Andrew Bartlett 
> Autobuild-Date(master): Thu Aug  9 08:20:36 CEST 2012 on sn-devel-104
> 
> commit ef87b4e4f10eb7d5974cb0e0861648d537153a00
> Author: Andrew Bartlett 
> Date:   Thu Aug 9 14:23:12 2012 +1000
> 
> s4-pydsdb: Provide control of if we should write index attributes when 
> reloading a schema
> 
> This allows us to carefully control the loading of the schema.
> 
> Andrew Bartlett
> 
> ---
> 
> Summary of changes:
>  source4/dsdb/pydsdb.c  |6 --
>  .../scripting/python/samba/provision/__init__.py   |7 ++-
>  source4/scripting/python/samba/samdb.py|8 
>  3 files changed, 14 insertions(+), 7 deletions(-)
> 
> 
> Changeset truncated at 500 lines:
> 
> diff --git a/source4/dsdb/pydsdb.c b/source4/dsdb/pydsdb.c
> index 3558049..9023d69 100644
> --- a/source4/dsdb/pydsdb.c
> +++ b/source4/dsdb/pydsdb.c
> @@ -873,7 +873,9 @@ static PyObject *py_dsdb_set_schema_from_ldb(PyObject 
> *self, PyObject *args)
>   struct ldb_context *from_ldb;
>   struct dsdb_schema *schema;
>   int ret;
> - if (!PyArg_ParseTuple(args, "OO", &py_ldb, &py_from_ldb))
> + char write_attributes = true;
> + if (!PyArg_ParseTuple(args, "OO|b",
> +   &py_ldb, &py_from_ldb, &write_attributes))
>   return NULL;
>  
>   PyErr_LDB_OR_RAISE(py_ldb, ldb);
> @@ -886,7 +888,7 @@ static PyObject *py_dsdb_set_schema_from_ldb(PyObject 
> *self, PyObject *args)
>   return NULL;
>   }
>  
> - ret = dsdb_reference_schema(ldb, schema, true);
> + ret = dsdb_reference_schema(ldb, schema, write_attributes);
>   PyErr_LDB_ERROR_IS_ERR_RAISE(py_ldb_get_exception(), ret, ldb);
>  
>   Py_RETURN_NONE;
> diff --git a/source4/scripting/python/samba/provision/__init__.py 
> b/source4/scripting/python/samba/provision/__init__.py
> index 192130e..94e857e 100644
> --- a/source4/scripting/python/samba/provision/__init__.py
> +++ b/source4/scripting/python/samba/provision/__init__.py
> @@ -1121,7 +1121,7 @@ def setup_samdb(path, session_info, provision_backend, 
> lp, names,
>  logger.info("Pre-loading the Samba 4 and AD schema")
>  
>  # Load the schema from the one we computed earlier
> -samdb.set_schema(schema)
> +samdb.set_schema(schema, write_attributes=False)
>  
>  # Set the NTDS settings DN manually - in order to have it already around
>  # before the provisioned tree exists and we connect
> @@ -1131,6 +1131,11 @@ def setup_samdb(path, session_info, provision_backend, 
> lp, names,
>  # DB
>  samdb.connect(path)
>  
> +# But we have to give it one more kick to have it use the schema
> +# during provision - it needs, now that it is connected, to write
> +# the schema @INDEX records to the database.
> +samdb.set_schema(schema, write_attributes=True)
> +
>  return samdb
>  
> 
> diff --git a/source4/scripting/python/samba/samdb.py 
> b/source4/scripting/python/samba/samdb.py
> index 7451b6c..3355e9a 100644
> --- a/source4/scripting/python/samba/samdb.py
> +++ b/source4/scripting/python/samba/samdb.py
> @@ -608,11 +608,11 @@ accountExpires: %u
>  def load_partition_usn(self, base_dn):
>  return dsdb._dsdb_load_partition_usn(self, base_dn)
>  
> -def set_schema(self, schema):
> -self.set_schema_from_ldb(schema.ldb)
> +def set_schema(self, schema, write_attributes=True):
> +self.set_schema_from_ldb(schema.ldb, 
> write_attributes=write_attributes)
>  
> -def set_schema_from_ldb(self, ldb_conn):
> -dsdb._dsdb_set_schema_from_ldb(self, ldb_conn)
> +def set_schema_from_ldb(self, ldb_conn, write_attributes=True):
> +dsdb._dsdb_set_schema_from_ldb(self, ldb_conn, write_attributes)
>  
>  def dsdb_DsReplicaAttribute(self, ldb, ldap_display_name, ldif_elements):
>  '''convert a list of attribute values to a DRSUAPI 
> DsReplicaAttribute'''
> 
> 

This co

Re: [SCM] Samba Shared Repository - branch master updated

[simo]

On Sat, 2012-06-16 at 10:14 +0200, Andrew Bartlett wrote: 
> The branch, master has been updated
>via  4edd8b8 s3-auth: Remove auth_netlogond
>via  9c715da s3-passdb: Remove pdb_ads

Andrew,
I would like you to revert these two commits ASAP.

Simo.

> via  d949736 s4-classicupgrade: Also ask testparm for 'smb passwd file'
>via  a0a2f79 WHATSNEW: Bump the version and announce the s3fs default
>via  d9f7195 s4-classicupgrade: Use "samba classic" description for 
> samba3 NT4-like domains in samba3upgrade
>via  39766b7 s4-lib/param: FLAG DAY for the default FILE SERVER
>via  b58dc18 s4-s3upgrade: Assert that administrator has a SID of 
> -500, and only skip root if it is -500
>via  61f7f01 s4-s3upgrade: Add my wins.dat and fix the parsing error
>via  d0b60f0 s4-s3upgrade: improve idmap import to use posixAccount 
> and posixGroup entries
>via  3c65bac s4-idmap: Add mapping using uidNumber and gidNumber like 
> idmap_ad
>   from  bbb7cbf Same fix as bug 8989 - Samba 3.5.x (and probably all 
> other versions of Samba) does not send correct responses to NT Transact 
> Secondary when no data and no params
> 
> http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master
> 
> 
> - Log -
> commit 4edd8b891a90a89a84fbfa3636cc568d247b04b2
> Author: Andrew Bartlett 
> Date:   Sun Jun 3 10:56:46 2012 +1000
> 
> s3-auth: Remove auth_netlogond
> 
> auth_netlogond was an important module in the development of the
> combined Samba 4.0, and was the first module to link smbd with the AD
> authentication store, showing that it was possible for NTLM
> authentication to be offloaded to the AD server components.
> 
> We now have auth_samba4, which provides the full GENSEC stack to smbd,
> which also matches exactly the group membership and privileges
> assignment and which is supported and tested as part of the official
> Samba 4.0 release configuration.
> 
> Andrew Bartlett
> 
> Autobuild-User(master): Andrew Bartlett 
> Autobuild-Date(master): Sat Jun 16 10:13:20 CEST 2012 on sn-devel-104
> 
> commit 9c715da1cbc256b9ae9298618c92807592607c9b
> Author: Andrew Bartlett 
> Date:   Sun Jun 3 10:54:06 2012 +1000
> 
> s3-passdb: Remove pdb_ads
> 
> pdb_ads was an important module in the development of the combined Samba 
> 4.0, and
> was the first module to show that standard samba3 tools such as smbpasswd 
> can be
> made to operate on the sam.ldb.
> 
> We now have pdb_samba4, which operates directly on the sam.ldb, rather 
> than via
> ldapi://, which uses transactions and which is supported and tested as 
> part
> of the official Samba 4.0 release configuration.
> 
> This module is not as complete (for example, it does not honour the idmap
> configuration) and requires that the samba binary be running to operate.
> 
> Andrew Bartlett
> 
> commit d949736f8dc02eec180723a55f4604b7b3aa83d8
> Author: Andrew Bartlett 
> Date:   Sat Jun 16 15:34:50 2012 +1000
> 
> s4-classicupgrade: Also ask testparm for 'smb passwd file'
> 
> commit a0a2f7999e20ab64dcbfca8299dbf0adfba0dea3
> Author: Andrew Bartlett 
> Date:   Sat Jun 16 13:12:50 2012 +1000
> 
> WHATSNEW: Bump the version and announce the s3fs default
> 
> commit d9f7195a1f5a12d5dc8865aa5553b61a4f770e3d
> Author: Andrew Bartlett 
> Date:   Sat Jun 16 13:06:44 2012 +1000
> 
> s4-classicupgrade: Use "samba classic" description for samba3 NT4-like 
> domains in samba3upgrade
> 
> commit 39766b75a40fbab73fc23dd947de44f8349ed466
> Author: Andrew Bartlett 
> Date:   Sat Jun 16 12:54:12 2012 +1000
> 
> s4-lib/param: FLAG DAY for the default FILE SERVER
> 
> This commit changes the default file server to be s3fs.  Existing
> installs wishing to keep the ntvfs file server need to set this in
> their smb.conf:
> 
> server services = +smb -s3fs
> dcerpc endpoint services = +winreg +srvsvc
> 
> Andrew Bartlett
> 
> commit b58dc1826e69c61a30d38b05e7f451404670baef
> Author: Andrew Bartlett 
> Date:   Sat Jun 16 14:19:42 2012 +1000
> 
> s4-s3upgrade: Assert that administrator has a SID of -500, and only skip 
> root if it is -500
> 
> Many upgraded installations have root as -1000, and so that account needs 
> to be kept.
> 
> Andrew Bartlett
> 
> commit 61f7f0155465b14612f7ac29a12c442ff25031b4
> Author: Andrew Bartlett 
> Date:   Sat Jun 16 13:58:06 2012 +1000
> 
> s4-s3upgrade: Add my wins.dat and fix the parsing error
> 
> The issue was that the numbers at the end of the lines are space
> padded.
> 
> Andrew Bartlett
> 
> commit d0b60f02dd3c324d4c990dae7334b228dddba075
> Author: Andrew Bartlett 
> Date:   Sun Jun 10 20:42:25 2012 +1000
> 
> s4-s3upgrade: improve idmap import to use posixAccount and posixGroup 
> entries
> 
> commit 3c65bac0b6fc104f4b

Remove unused finddcs_nbt (Re: [SCM] Samba Shared Repository - branch master updated)

[Stefan (metze) Metzmacher]

Hi Andrew,

> commit 06c90cb6f55701effa4cbafaf189a4de8471949b
> Author: Andrew Bartlett 
> Date:   Thu Jun 7 14:21:25 2012 +1000
> 
> s4-libcli: Remove unused finddcs_nbt
> 
> This would only do the NBT getdc lookup for a single DC (but would
> find multiple DCs at first stage), but more particular it of course
> uses Netbios rather than DNS names.
> 
> In any case it was also unused, as we use CLDAP for reliable DC
> location these days.
> 
> Found by callcatcher

I think it's a bug that it's not used.

Samba3 DCs doesn't support CLDAP.

metze



signature.asc
Description: OpenPGP digital signature

Re: [SCM] Samba Shared Repository - branch master updated

[simo]

On Wed, 2012-04-25 at 11:44 +0200, Andrew Bartlett wrote:
> commit b8055132b1c62dd19981fea2822ab9e1829a8ded
> Author: Andrew Bartlett 
> Date:   Wed Apr 25 17:53:18 2012 +1000
> 
> s4-messaging: Use generate_random() to get a unique ID for
> messaging clients
> 
> The call to random() resulted in duplicate values for s3fs
> configurations
> which, due to the forked child, all started with the same random
> seed.
> 
> A future improvement would be to move to a proven unique value.
> 
> Andrew Bartlett
> 
> Autobuild-User: Andrew Bartlett 
> Autobuild-Date: Wed Apr 25 11:43:40 CEST 2012 on sn-devel-104


Andrew are you sure you need to use /dev/urandom here ? It doesn't look
like you need absolutelu unpredictable numbers here, just non-colliding
numbers.
You changed the code to draw from urandom, and if it is used often it
mean it will suck a lot of entropy out of the system, causing any
application that need to use /dev/random to halt.
Wouldn't it have been simpler to just run srand(time(NULL)*pid) to get a
new seed for the process ?

Simo.

-- 
Simo Sorce
Samba Team GPL Compliance Officer 
Principal Software Engineer at Red Hat, Inc. 

Re: [SCM] Samba Shared Repository - branch master updated

[simo]

On Tue, 2012-04-10 at 11:15 +0200, Andrew Bartlett wrote:
> index cafc020..f07bbc9 100644
> --- a/lib/ldb/include/ldb_private.h
> +++ b/lib/ldb/include/ldb_private.h
> @@ -181,4 +181,12 @@ struct ldb_val ldb_binary_decode(TALLOC_CTX
> *mem_ctx, const char *str);
>  const char *ldb_options_find(struct ldb_context *ldb, const char
> *options[],
>  const char *option_name);
>  
> +struct ldif_read_file_state {
> +   FILE *f;
> +   size_t line_no;
> +};

Andrew,
can you put this back in ldb_ldif.c and provide a getter function for
the line number instead ?
Let's avoid keeping spreading structures all over.

> +struct ldb_ldif *ldb_ldif_read_file_state(struct ldb_context *ldb, 
> + struct ldif_read_file_state
> *state);
> + 
-- 
Simo Sorce
Samba Team GPL Compliance Officer 
Principal Software Engineer at Red Hat, Inc. 

Re: [SCM] Samba Shared Repository - branch master updated

[Jeremy Allison]

On Mon, Apr 09, 2012 at 09:53:25PM +0200, Jelmer Vernooij wrote:
> Hi Jeremy,
> 
> Am 09/04/12 21:41, schrieb Jeremy Allison:
> > The branch, master has been updated
> >via  5701a4d Move blksize_t and blkcnt_t to replace.h from 
> > includes.h. Should help with platforms that don't have these.
> >   from  5184f41 libreplace: We have a poll replacement based on select
> >
> > http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master
> >
> >
> > - Log -
> > commit 5701a4d8611ad1e310b76b00b146b0902c9b34ff
> > Author: Jeremy Allison 
> > Date:   Mon Apr 9 11:04:07 2012 -0700
> >
> > Move blksize_t and blkcnt_t to replace.h from includes.h. Should help 
> > with platforms that don't have these.
> > 
> Thanks for moving these to libreplace. It'd be great if you can also
> move the relevant tests from configure.in / wscript to the equivalent
> files in lib/replace, so libreplace stays self-contained.

Hmmm, ok. This is probably easy for wscript, but I'm not sure
what to change for configure.in.

Jeremy.

Re: [SCM] Samba Shared Repository - branch master updated

[Jelmer Vernooij]

Hi Jeremy,

Am 09/04/12 21:41, schrieb Jeremy Allison:
> The branch, master has been updated
>via  5701a4d Move blksize_t and blkcnt_t to replace.h from includes.h. 
> Should help with platforms that don't have these.
>   from  5184f41 libreplace: We have a poll replacement based on select
>
> http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master
>
>
> - Log -
> commit 5701a4d8611ad1e310b76b00b146b0902c9b34ff
> Author: Jeremy Allison 
> Date:   Mon Apr 9 11:04:07 2012 -0700
>
> Move blksize_t and blkcnt_t to replace.h from includes.h. Should help 
> with platforms that don't have these.
> 
Thanks for moving these to libreplace. It'd be great if you can also
move the relevant tests from configure.in / wscript to the equivalent
files in lib/replace, so libreplace stays self-contained.

Cheers,

Jelmer



signature.asc
Description: OpenPGP digital signature

Re: [SCM] Samba Shared Repository - branch master updated

[Jelmer Vernooij]

Hi Andreas, Björn,

On 02/09/2012 04:40 PM, Andreas Schneider wrote:

The branch, master has been updated
via  5ec1273 s3-printing: Add new printers to registry.
   from  2464a7b s3-libsmb: Remove obsolete smb_krb5_locate_kdc.

http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master


- Log -
commit 5ec12732c3092d248a374ae7af155a409c7ee88e
Author: Björn Baumbach
Date:   Tue Feb 7 11:41:54 2012 +0100

 s3-printing: Add new printers to registry.

 This fixes bug #8554, #8612 and #8748.

 Pair-Programmed-With: Stefan Metzmacher

 Autobuild-User: Andreas Schneider
 Autobuild-Date: Thu Feb  9 16:39:04 CET 2012 on sn-devel-104

After this change (or perhaps an earlier one?) I'm unable to push 
changes to autobuild. The last two builds fail with:


[275/1226 in 17m41s] samba3.posix_s3.rpc.spoolss.printer (s3dc)
Testing DsSpooler<->  SetPrinter relations
Testing GetPrinter level 2
Testing SetPrinter level 2
Testing GetPrinterDataEx(DsSpooler - description)
Testing SetPrinter level 2
Testing GetPrinterDataEx(DsSpooler - location)
Testing GetPrinter level 2
Testing GetPrinterDataEx(DsSpooler - description)
Testing GetPrinterDataEx(DsSpooler - driverName)
WARNING!: ../source4/torture/rpc/spoolss.c:3648: r.out.result was WERR_BADFILE, 
expected WERR_OK: GetPrinterDataEx(DsSpooler - driverName) failed
WARNING!: ../source4/torture/rpc/spoolss.c:5509: Expression `test_GetPrinterDataEx(tctx, p, handle, 
"DsSpooler", "driverName",&type,&data,&needed)' failed: failed to query
UNEXPECTED(failure): samba3.posix_s3.rpc.spoolss.printer 
.addprinterex.printerdata_dsspooler
REASON: _StringException: _StringException: ../source4/torture/rpc/spoolss.c:7879: 
Expression `test_PrinterData_DsSpooler(tctx, p,&t->handle, 
t->info2.printername)' failed: failed to test printer data winreg dsspooler

FAILED (1 failures, 0 errors and 0 unexpected successes in 0 testsuites)

A summary with detailed information can be found in:
  ./bin/ab/summary
test: running (/usr/bin/perl /memdisk/autobuild/flakey/b12981/samba4/selftest/selftest.pl --target=samba 
--prefix=./bin/ab --srcdir=/memdisk/autobuild/flakey/b12981/samba4 
--exclude=/memdisk/autobuild/flakey/b12981/samba4/selftest/skip --testlist="/usr/bin/python 
/memdisk/autobuild/flakey/b12981/samba4/source3/selftest/tests.py|" --testlist="/usr/bin/python 
/memdisk/autobuild/flakey/b12981/samba4/source4/selftest/tests.py|" 
--binary-mapping=nmblookup3:nmblookup3,smbclient3:smbclient3,smbtorture4:smbtorture,ntlm_auth3:ntlm_auth3 
--exclude=/memdisk/autobuild/flakey/b12981/samba4/selftest/slow --socket-wrapper&&  touch 
./bin/ab/st_done) | /usr/bin/python -u /memdisk/autobuild/flakey/b12981/samba4/selftest/filter-subunit 
--expected-failures=/memdisk/autobuild/flakey/b12981/samba4/selftest/knownfail 
--flapping=/memdisk/autobuild/flakey/b12981/samba4/selftest/flapping --fail-immediately | tee 
./bin/ab/subunit | /usr/bin/python -u /memdisk/autobuild/flakey/b12981/samba4/selftest/format-subunit 
--prefix=./bin/ab --immediate

Cheers,

Jelmer

Re: [SCM] Samba Shared Repository - branch master updated

[Jelmer Vernooij]

Hi Tridge, Amitay,

On 10/11/11 06:05, Andrew Tridgell wrote:

The branch, master has been updated

via  ac0bba2 test: added -D option to subunitrun
subunitrun is to be deprecated, "python -m subunit.run" is the preferred 
way to run tests. Can we revert this?



via  333248f pyldb: fixed ldb API test
This makes LDB depend on Samba, because it imports from samba.tests. 
This makes it impossible to run the pyldb testsuite without having Samba 
installed. I'll submit an alternative fix.


Cheers,

Jelmer

Re: [SCM] Samba Shared Repository - branch master updated

[Jelmer Vernooij]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Hi Matthieu,

On 02/11/11 15:15, Matthieu Patou wrote:
>
> +#tests on sites
> +class SimpleSitesTests(SitesBaseTests):
> +
> +
> + def test_create(self):
> + """test creation of 1 site"""
> +
> + self.ldb_admin.transaction_start()
> + ok = sites.create_site(self.ldb_admin,
self.ldb_admin.get_config_basedn(),
> + "testsamba")
> + self.ldb_admin.transaction_commit()
> + self.assertTrue(ok)
> + ok = False
> + try:
> + ok = sites.create_site(self.ldb_admin,
self.ldb_admin.get_config_basedn(),
> + "testsamba")
> + self.assertFalse(ok)
> + except:
> + self.assertFalse(ok)
This looks dodgy. If sites.create_site raises *any* exception (even a
SyntaxError!), then this test still passes since ok is False by default.

It's almost never right to use "except:" - and if you do use it, please
use "raise" to re-raise the exception. "except:" catches every kind of
exception, including KeyboardInterrupt and SyntaxError.

> diff --git a/source4/scripting/python/samba/netcmd/sites.py
b/source4/scripting/python/samba/netcmd/sites.py
> new file mode 100644
> index 000..a63b524
> --- /dev/null
> +++ b/source4/scripting/python/samba/netcmd/sites.py
> @@ -0,0 +1,96 @@
> +
> +#!/usr/bin/env python
> +#
> +# sites management
> +#
> +# Copyright Matthieu Patou  2011
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program. If not, see .
> +#
> +
> +
> +
> +import os
> +from samba import sites
> +from samba import Ldb
> +from samba.auth import system_session
> +from samba.netcmd import (
> + Command,
> + CommandError,
> + SuperCommand
> + )
> +
> +
> +class cmd_sites_create(Command):
> + """Create a new site"""
> +
> + synopsis = "%prog  [options]"
> +
> + takes_args = ["sitename"]
> +
> + def run(self, sitename, sambaopts=None, credopts=None, versionopts=None):
> + lp = sambaopts.get_loadparm()
> + creds = credopts.get_credentials(lp, fallback_machine=True)
> + name = "sam.ldb"
> + path = lp.get("private dir")
> + url = os.path.join(path, name)
It should be shorter to use lp.private_path("sam.ldb").

>
> + if not os.path.exists(url):
> + raise CommandError("secret database not found at %s " % url)
This says secrets database, but it looks like it's actually about the sam ?

>
> + samdb = Ldb(url=url, session_info=system_session(),
> + credentials=creds, lp=lp)
You probably want to use SamDB rather than Ldb here.
>
> +
> + samdb.transaction_start()
> + ok = sites.create_site(samdb, samdb.get_config_basedn(), sitename)
> + samdb.transaction_commit()
This needs a try/except/finally otherwise we'll get a nasty warning
about transactions that weren't closed.
>
> +class cmd_sites_delete(Command):
> + """Delete a new site"""
> +
The same comments as mentioned above apply to cmd_sites_delete.

>
> diff --git a/source4/scripting/python/samba/sites.py
b/source4/scripting/python/samba/sites.py
> new file mode 100644
> index 000..d1d0e75
> --- /dev/null
> +++ b/source4/scripting/python/samba/sites.py
> @@ -0,0 +1,63 @@
> +#!/usr/bin/env python
> +#
> +# python site manipulation code
> +# Copyright Matthieu Patou  2011
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program. If not, see .
> +#
> +
> +"""Manipulating sites."""
> +
> +import ldb
> +from ldb import FLAG_MOD_ADD
> +
> +def create_site(samdb, configDn, siteName):
It would be nice to have a docstring on this method. It seems to always
return True, but what does that actually indicate? It might be nicer to
not return anything specific but just raise exceptions in case problems
occur.

Cheers,

Jelmer
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQIcBAEBAgAGBQJOsVKTAAoJEACAbyvXKaRXewoP/inmGReeG3tiz8pt9kXcrLNh
dyFQEJf/8ckiGpgqZRT8wbqUgnq83VdXdg+6k0x8aPH4aNR+kvZ3v9iBSMtM9U

Re: [SCM] Samba Shared Repository - branch master updated

[simo]

On Fri, 2011-10-28 at 11:35 +0200, Andreas Schneider wrote:
> +Requires=smb.service nmb.service
> +After=syslog.target network.target smb.service nmb.service

This looks wrong, winbind does not require smb or nmb to run, if you
have a laptop and do not offer shares you may want to run just winbind
and no smb or nmb service.
And actually I think you probably want to start winbind before smb *if*
you are using winbind on a samba file server.

Simo.

-- 
Simo Sorce
Samba Team GPL Compliance Officer 
Principal Software Engineer at Red Hat, Inc. 

Re: [SCM] Samba Shared Repository - branch master updated

[Matthieu Patou]

Hello Andrew,

On two of my machines with this patchset I have this error:

samba version 4.0.0alpha18-DEVELOPERBUILD started.
Copyright Andrew Tridgell and the Samba Team 1992-2011
Called with maxruntime 7500 - current ts 1318577424
samba: using 'standard' process model
Failed to start service 'rpc' - NT_STATUS_INVALID_SYSTEM_SERVICE
Starting Services failed - NT_STATUS_INVALID_SYSTEM_SERVICE
./bin/samba failed with status 1!
Lookup failed - NT_STATUS_IO_TIMEOUT
Lookup failed - NT_STATUS_HOST_UNREACHABLE
Lookup failed - NT_STATUS_IO_TIMEOUT
Lookup failed - NT_STATUS_HOST_UNREACHABLE
Lookup failed - NT_STATUS_IO_TIMEOUT
Lookup failed - NT_STATUS_HOST_UNREACHABLE
Lookup failed - NT_STATUS_IO_TIMEOUT
Lookup failed - NT_STATUS_HOST_UNREACHABLE
Lookup failed - NT_STATUS_IO_TIMEOUT
Lookup failed - NT_STATUS_HOST_UNREACHABLE
Lookup failed - NT_STATUS_IO_TIMEOUT
Lookup failed - NT_STATUS_HOST_UNREACHABLE
SAMBA LOG of: LOCALDC
samba version 4.0.0alpha18-DEVELOPERBUILD started.
Copyright Andrew Tridgell and the Samba Team 1992-2011
Called with maxruntime 7500 - current ts 1318577424
samba: using 'standard' process model
Failed to start service 'rpc' - NT_STATUS_INVALID_SYSTEM_SERVICE
Starting Services failed - NT_STATUS_INVALID_SYSTEM_SERVICE
./bin/samba failed with status 1!

I bissect and found that it's the first patch 
b21b012756dbb9e7022280b34d7103a5dcbea6d6 that is causing the error.


Can you have a look ?



On 11/10/2011 06:14, Andrew Bartlett wrote:

The branch, master has been updated
via  4549862 gensec: trim header includes back to what is actually 
required
via  534355f auth/credentials Declare remaining functions are public 
interfaces and put into credentials.h
via  fe02752 auth: move gensec_start.c to the top level
via  561d834 auth: move credentials layer to the top level
via  1255383 s4-s3-upgrade: Allow import (just without a uid mapping) 
where getpwnam fails
via  c9bb497 lib/param Avoid dyn_ defines that are not provided by the 
autoconf build
via  1e5af9e lib/param Use strcasecmp_m rather than strcasecmp as this 
is banned in the s3 includes.h
via  8f2741b lib/param Use talloc_strdup rather than strdup as strdup 
is banned in the s3 includes.h
via  1b81af0 lib/param Avoid the name string_set as this is already 
used in the s3 param code
via  15c97a8 lib/param Avoid the use of the name service_ok() which is 
used in the s3 param code
via  d0ecd1a lib/param: Remove unused #include of lib/socket/socket.h
via  26de383 libcli/smb Move CSC_POLICY_* definition to smb_constants.h
via  b21b012 lib/param move source4 param code to the top level
   from  6bed577 pac: Fix wrong memory allocation check

http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master


- Log -
commit 454986298aa5696b0b029e2feba0109617aaf968
Author: Andrew Bartlett
Date:   Sun Oct 9 23:28:15 2011 +1100

 gensec: trim header includes back to what is actually required

 Autobuild-User: Andrew Bartlett
 Autobuild-Date: Tue Oct 11 06:13:08 CEST 2011 on sn-devel-104

commit 534355fecf5a14a36ec5a3d643bcf2140df2da4e
Author: Andrew Bartlett
Date:   Sun Oct 9 23:27:44 2011 +1100

 auth/credentials Declare remaining functions are public interfaces and put 
into credentials.h

 This is in preperation for this file being used by s3, and recognises that 
these are all
 reasonable, public interfaces but were not declared as such in the past.

 Andrew Bartlett

commit fe02752ed6493efb7af28faa3d64d9fd7895d6f1
Author: Andrew Bartlett
Date:   Fri Oct 7 17:24:12 2011 +1100

 auth: move gensec_start.c to the top level

 This does not change who uses gensec for now, but makes it possible to
 write new gensec modules outside source4/

 Andrew Bartlett

commit 561d834123a2a8a96954f7cca556f8838ab38b72
Author: Andrew Bartlett
Date:   Fri Oct 7 17:20:33 2011 +1100

 auth: move credentials layer to the top level

 This will allow gensec_start.c to move to the top level.  This does not 
change
 what code uses the cli_credentials code, but allows the gensec code to be
 more broadly.

 Andrew Bartlett

commit 1255383140a9b3fbd957c1f7ce47e89c17cc4eda
Author: Andrew Bartlett
Date:   Mon Oct 10 13:09:30 2011 +1100

 s4-s3-upgrade: Allow import (just without a uid mapping) where getpwnam 
fails

 This allows the tests to pass on systems without a jelmer user :-)

 Andrew Bartlett

commit c9bb497f3f7fae8aa6ec4a4a45a2ac4047b640a5
Author: Andrew Bartlett
Date:   Sun Oct 9 23:25:11 2011 +1100

 lib/param Avoid dyn_ defines that are not provided by the autoconf build

 The autoconf build will never use these parameters or load the
 smb.conf with these defaults, so the defaults are not important.

 Andrew Bartlett

commit 1e5af9ecd0567e0afbe29ee3d69d4537628a3d63
Author: Andrew Bartlett
Date:   Sun Oct 9 23:24:

Re: [SCM] Samba Shared Repository - branch master updated

[Michael Adam]

Hi folks,

I am currently working on adding ctdb and clustered samba build
to selftest and autobuild.

I have added a ctdb snapshot of current ctdb master into the
master branch. I can build ctdb and samba with cluster support.
Now I am working on adding a clusteredmember target to Samba3.

Eventually, I think we should add ctdb master with full history
to samba master and change to developing it here. But for a
start/poc, I'll just add a snapshot, see:

http://gitweb.samba.org/?p=obnox/samba/samba-obnox.git;a=shortlog;h=refs/heads/master-ctdbtest

Cheers - Michael



Michael Adam wrote:
> The branch, master has been updated
>via  71f5f41 s3:dbwrap_ctdb: fix the build after changing signature of 
> tdb_wrap_open()
>   from  e287a72 add lib/param files to .gitignore
> 
> http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master
> 
> 
> - Log -
> commit 71f5f41860e678e6151b969a05729ccf2d738e35
> Author: Michael Adam 
> Date:   Thu Oct 13 16:50:57 2011 +0200
> 
> s3:dbwrap_ctdb: fix the build after changing signature of tdb_wrap_open()
> 
> Autobuild-User: Michael Adam 
> Autobuild-Date: Thu Oct 13 18:50:47 CEST 2011 on sn-devel-104
> 
> ---
> 
> Summary of changes:
>  source3/lib/dbwrap/dbwrap_ctdb.c |8 +++-
>  1 files changed, 7 insertions(+), 1 deletions(-)
> 
> 
> Changeset truncated at 500 lines:
> 
> diff --git a/source3/lib/dbwrap/dbwrap_ctdb.c 
> b/source3/lib/dbwrap/dbwrap_ctdb.c
> index 4c5cd64..e0f98cc 100644
> --- a/source3/lib/dbwrap/dbwrap_ctdb.c
> +++ b/source3/lib/dbwrap/dbwrap_ctdb.c
> @@ -23,6 +23,7 @@
>  #include "lib/util/tdb_wrap.h"
>  #include "util_tdb.h"
>  #include "dbwrap/dbwrap_rbt.h"
> +#include "lib/param/param.h"
>  
>  #ifdef CLUSTER_SUPPORT
>  
> @@ -1428,6 +1429,7 @@ struct db_context *db_open_ctdb(TALLOC_CTX *mem_ctx,
>   struct db_ctdb_ctx *db_ctdb;
>   char *db_path;
>   struct ctdbd_connection *conn;
> + struct loadparm_context *lp_ctx;
>  
>   if (!lp_clustering()) {
>   DEBUG(10, ("Clustering disabled -- no ctdb\n"));
> @@ -1474,7 +1476,11 @@ struct db_context *db_open_ctdb(TALLOC_CTX *mem_ctx,
>   chmod(db_path, mode);
>   }
>  
> - db_ctdb->wtdb = tdb_wrap_open(db_ctdb, db_path, hash_size, tdb_flags, 
> O_RDWR, 0);
> + lp_ctx = loadparm_init_s3(db_path, loadparm_s3_context());
> +
> + db_ctdb->wtdb = tdb_wrap_open(db_ctdb, db_path, hash_size, tdb_flags,
> +   O_RDWR, 0, lp_ctx);
> + talloc_unlink(db_path, lp_ctx);
>   if (db_ctdb->wtdb == NULL) {
>   DEBUG(0, ("Could not open tdb %s: %s\n", db_path, 
> strerror(errno)));
>   TALLOC_FREE(result);
> 
> 
> -- 
> Samba Shared Repository


pgpdYZ8iYKSUo.pgp
Description: PGP signature

Re: [SCM] Samba Shared Repository - branch master updated

[Guenther Deschner]

Hi David,

On 09/29/2011 02:48 PM, David Disseldorp wrote:

The branch, master has been updated
via  08573c2 s4: add SMB2_FSCTL opcodes
via  dbcd59f s3-smb2_server: fix ioctl InputOffset checking
via  1848295 s3-smb2_server: SMB2_OP_IOCTL doesn't require at least 1 
dyn byte
via  392fd0d s4-torture: add smb2 ioctl test suite
via  bd5e975 s4-torture: remove unchecked read from smb2 create
   from  27195b3 socket_wrapper: correctly handle dup()/dup2() ref counting

http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master


Congratulations to your first commit (not your first patch of course) !

And welcome on board!

Cheers,
Guenther
--
Günther DeschnerGPG-ID: 8EE11688
Red Hat gdesch...@redhat.com
Samba Team  g...@samba.org

Re: [SCM] Samba Shared Repository - branch master updated

[Stefan (metze) Metzmacher]

Am 23.09.2011 07:01, schrieb Matthieu Patou:
> On 22/09/2011 21:14, Stefan (metze) Metzmacher wrote:
>> Hi Matthieu,
>>
>>> diff --git a/source4/ntvfs/posix/python/pyxattr_tdb.c
>>> b/source4/ntvfs/posix/python/pyxattr_tdb.c
>>> index 2b28aca..a08557e 100644
>>> --- a/source4/ntvfs/posix/python/pyxattr_tdb.c
>>> +++ b/source4/ntvfs/posix/python/pyxattr_tdb.c
>>> @@ -60,9 +60,8 @@ static PyObject *py_wrap_setxattr(PyObject *self,
>>> PyObject *args)
>>>   status = push_xattr_blob_tdb_raw(eadb, mem_ctx, attribute,
>>> filename, -1,
>>>   &blob);
>>>   if (!NT_STATUS_IS_OK(status)) {
>>> -PyErr_FromNTSTATUS(status);
>>>   talloc_free(mem_ctx);
>>> -return NULL;
>>> +PyErr_NTSTATUS_IS_ERR_RAISE(status);
>>>   }
>>>   talloc_free(mem_ctx);
>>>   Py_RETURN_NONE;
>>> @@ -91,9 +90,8 @@ static PyObject *py_wrap_getxattr(PyObject *self,
>>> PyObject *args)
>>>   status = pull_xattr_blob_tdb_raw(eadb, mem_ctx, attribute,
>>> filename,
>>>-1, 100,&blob);
>>>   if (!NT_STATUS_IS_OK(status) || blob.length<  0) {
>>> -PyErr_FromNTSTATUS(status);
>>>   talloc_free(mem_ctx);
>>> -return NULL;
>>> +PyErr_NTSTATUS_IS_ERR_RAISE(status);
>> Why this changes? What was the problem?
>>
>> NT_STATUS_IS_ERR() is not the same as !NT_STATUS_IS_OK()
> Well the problem is that without this if your status was != OK you'll
> get error like instead of getting the right exception.
> "ERROR(): uncaught exception - error
> return without exception set"
> 
> 
> As PyErr_FromNTSTATUS(status) is not the same as PyErr_SetNTSTATUS(status)
> 
> If _IS_ERR is not the same then we need to have this code:
> if (!NT_STATUS_IS_OK(status)) {
> PyErr_SetNTSTATUS(status);
> talloc_free(mem_ctx);
> return NULL;
> }

yes, that looks more sane to me.

metze



signature.asc
Description: OpenPGP digital signature

Re: [SCM] Samba Shared Repository - branch master updated

[Matthieu Patou]

On 22/09/2011 21:14, Stefan (metze) Metzmacher wrote:

Hi Matthieu,


diff --git a/source4/ntvfs/posix/python/pyxattr_tdb.c 
b/source4/ntvfs/posix/python/pyxattr_tdb.c
index 2b28aca..a08557e 100644
--- a/source4/ntvfs/posix/python/pyxattr_tdb.c
+++ b/source4/ntvfs/posix/python/pyxattr_tdb.c
@@ -60,9 +60,8 @@ static PyObject *py_wrap_setxattr(PyObject *self, PyObject 
*args)
status = push_xattr_blob_tdb_raw(eadb, mem_ctx, attribute, filename, -1,
&blob);
if (!NT_STATUS_IS_OK(status)) {
-   PyErr_FromNTSTATUS(status);
talloc_free(mem_ctx);
-   return NULL;
+   PyErr_NTSTATUS_IS_ERR_RAISE(status);
}
talloc_free(mem_ctx);
Py_RETURN_NONE;
@@ -91,9 +90,8 @@ static PyObject *py_wrap_getxattr(PyObject *self, PyObject 
*args)
status = pull_xattr_blob_tdb_raw(eadb, mem_ctx, attribute, filename,
 -1, 
100,&blob);
if (!NT_STATUS_IS_OK(status) || blob.length<  0) {
-   PyErr_FromNTSTATUS(status);
talloc_free(mem_ctx);
-   return NULL;
+   PyErr_NTSTATUS_IS_ERR_RAISE(status);

Why this changes? What was the problem?

NT_STATUS_IS_ERR() is not the same as !NT_STATUS_IS_OK()
Well the problem is that without this if your status was != OK you'll 
get error like instead of getting the right exception.
"ERROR(): uncaught exception - error 
return without exception set"



As PyErr_FromNTSTATUS(status) is not the same as PyErr_SetNTSTATUS(status)

If _IS_ERR is not the same then we need to have this code:
if (!NT_STATUS_IS_OK(status)) {
PyErr_SetNTSTATUS(status);
talloc_free(mem_ctx);
return NULL;
}

If you have no objection I'll push a fix like that.

--
Matthieu Patou
Samba Team
http://samba.org

Re: [SCM] Samba Shared Repository - branch master updated

[Stefan (metze) Metzmacher]

Hi Matthieu,

> diff --git a/source4/ntvfs/posix/python/pyxattr_tdb.c 
> b/source4/ntvfs/posix/python/pyxattr_tdb.c
> index 2b28aca..a08557e 100644
> --- a/source4/ntvfs/posix/python/pyxattr_tdb.c
> +++ b/source4/ntvfs/posix/python/pyxattr_tdb.c
> @@ -60,9 +60,8 @@ static PyObject *py_wrap_setxattr(PyObject *self, PyObject 
> *args)
>   status = push_xattr_blob_tdb_raw(eadb, mem_ctx, attribute, filename, -1,
>&blob);
>   if (!NT_STATUS_IS_OK(status)) {
> - PyErr_FromNTSTATUS(status);
>   talloc_free(mem_ctx);
> - return NULL;
> + PyErr_NTSTATUS_IS_ERR_RAISE(status);
>   }
>   talloc_free(mem_ctx);
>   Py_RETURN_NONE;
> @@ -91,9 +90,8 @@ static PyObject *py_wrap_getxattr(PyObject *self, PyObject 
> *args)
>   status = pull_xattr_blob_tdb_raw(eadb, mem_ctx, attribute, filename, 
>-1, 
> 100, &blob);
>   if (!NT_STATUS_IS_OK(status) || blob.length < 0) {
> - PyErr_FromNTSTATUS(status);
>   talloc_free(mem_ctx);
> - return NULL;
> + PyErr_NTSTATUS_IS_ERR_RAISE(status);

Why this changes? What was the problem?

NT_STATUS_IS_ERR() is not the same as !NT_STATUS_IS_OK()

metze



signature.asc
Description: OpenPGP digital signature

Re: [SCM] Samba Shared Repository - branch master updated

[Jeremy Allison]

On Wed, Jul 06, 2011 at 03:41:02PM +0200, Stefan Metzmacher wrote:
> The branch, master has been updated
>via  f9601a9 s3:sharesec: return an error if get_share_security() 
> returns NULL
>   from  93ad34c s3:test: catch more errors in test_access_check() in the 
> smbclient_s3 test
> 
> http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master
> 
> 
> - Log -
> commit f9601a91805651963834e37e27800a74931468b7
> Author: Stefan Metzmacher 
> Date:   Wed Jul 6 12:31:41 2011 +0200
> 
> s3:sharesec: return an error if get_share_security() returns NULL
> 
> This indicates an error, as it uses get_share_security_default()
> if no security descriptor is configured.
> 
> Jeremy, please check.

+1 from me - this fixes an error condition. Thanks !

Re: [SCM] Samba Shared Repository - branch master updated

[Jelmer Vernooij]

On Mon, 2010-05-31 at 09:46 +0200, Stefan (metze) Metzmacher wrote:
> > diff --git a/lib/talloc/wscript b/lib/talloc/wscript
> > index 3a87506..61930f6 100644
> > --- a/lib/talloc/wscript
> > +++ b/lib/talloc/wscript
> > @@ -60,14 +60,14 @@ def build(bld):
> >abi_file='ABI/talloc-%s.sigs' % VERSION,
> >abi_match='talloc* _talloc*',
> >hide_symbols=True,
> > -  vnum=VERSION)
> > +  vnum=VERSION, is_bundled=True)
> >  
> >  # should we also install the symlink to libtalloc1.so here?
> >  bld.SAMBA_LIBRARY('talloc-compat1',
> >'compat/talloc_compat1.c',
> >deps='talloc',
> >enabled = bld.env.TALLOC_COMPAT1,
> > -  vnum=VERSION)
> > +  vnum=VERSION, is_bundled=True)
> >
> 
> shouldn't we use 'not bld.env.standalone_talloc' instead of 'True'
> here too?
That's a good point, thanks. Fixed in master.

Cheers,

Jelmer


signature.asc
Description: This is a digitally signed message part

Re: [SCM] Samba Shared Repository - branch master updated

[Jelmer Vernooij]

On Mon, 2010-05-31 at 19:49 +0200, Stefan (metze) Metzmacher wrote:
> Am 31.05.2010 19:23, schrieb Jelmer Vernooij:
> > The branch, master has been updated
> >via  7f75ee0... ldb: Install ldb_handlers.h header.
> >via  fe8302b... ldb: Remove Samba-specific symbols.
> >via  1bc53f0... ldb: Move utility functions to separate file.
> >via  82d56b9... ldb: Fix dependencies when building with system ldb.
> >   from  471ed70... s3:smbd map_username() doesn't need sconn anymore
> > 
> > http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master
> > 
> > 
> > - Log -
> > commit 7f75ee025ff9c02763fb9201d94af4b2739c8e03
> > Author: Jelmer Vernooij 
> > Date:   Mon May 31 18:29:11 2010 +0200
> > 
> > ldb: Install ldb_handlers.h header.
> > 
> > commit fe8302b235197e359a20ba0489eb72b54793963e
> > Author: Jelmer Vernooij 
> > Date:   Mon May 31 18:28:09 2010 +0200
> > 
> > ldb: Remove Samba-specific symbols.
> > 
> > commit 1bc53f0213b04e30ebf2ac60695c84c5781e4e72
> > Author: Jelmer Vernooij 
> > Date:   Mon May 31 18:12:05 2010 +0200
> > 
> > ldb: Move utility functions to separate file.
> > 
> > commit 82d56b937445585b9e60451daf660aee5ace0523
> > Author: Jelmer Vernooij 
> > Date:   Mon May 31 16:05:41 2010 +0200
> > 
> > ldb: Fix dependencies when building with system ldb.
> 
> I'm getting this again:
> 
> [1862/1965] abi_check: source4/bin/default/source4/lib/ldb/libldb-samba4.so
> libldb-samba4.so: symbol ldb_wrap_connect has been added - please mark
> it _PRIVATE_ or update minor version
> signature: struct ldb_context *(TALLOC_CTX *, struct
> tevent_context *, struct loadparm_context *, const char *, struct
> auth_session_info *, struct cli_credentials *, unsigned int)
> libldb-samba4.so: symbol ldb_register_samba_handlers has been added -
> please mark it _PRIVATE_ or update minor version
> signature: int (struct ldb_context *)
> libldb-samba4.so: symbol ldb_wrap_fork_hook has been added - please mark
> it _PRIVATE_ or update minor version
> signature: void (void)
> libldb-samba4.so: symbol ldb_samba_syntax_by_lDAPDisplayName has been
> added - please mark it _PRIVATE_ or update minor version
> signature: const struct ldb_schema_syntax *(struct ldb_context
> *, const char *)
> libldb-samba4.so: symbol ldb_samba_syntax_by_name has been added -
> please mark it _PRIVATE_ or update minor version
> signature: const struct ldb_schema_syntax *(struct ldb_context
> *, const char *)
> Waf: Leaving directory `/home/metze/master4/source4/bin'
> Build failed: ABI for libldb-samba4.so has changed - please fix library
> version then build with --abi-update
> See http://wiki.samba.org/index.php/Waf#ABI_Checking for more information
> make: *** [all] Fehler 1
Have you tried a "git clean -f -x -d" ?

I can't reproduce the issue with that, and as far as I can tell the
buildfarm is happy too.

Cheers,

Jelmer


signature.asc
Description: This is a digitally signed message part

Re: [SCM] Samba Shared Repository - branch master updated

[Jelmer Vernooij]

Hi Matthias,

On Fri, 2010-05-28 at 12:08 -0500, Matthias Dieter Wallnöfer wrote:
> Summary of changes:
>  source4/web_server/wsgi.c |3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> 
> Changeset truncated at 500 lines:
> 
> diff --git a/source4/web_server/wsgi.c b/source4/web_server/wsgi.c
> index 7a23ae4..db1a5f2 100644
> --- a/source4/web_server/wsgi.c
> +++ b/source4/web_server/wsgi.c
> @@ -301,7 +301,8 @@ static PyObject *create_environ(bool tls, int 
> content_length, struct http_header
>   PyDict_SetItemString(env, "CONTENT_TYPE", 
> PyString_FromString(hdr->value));
>   } else { 
>   if (asprintf(&name, "HTTP_%s", hdr->name) < 0) {
> - continue;
> + PyErr_NoMemory();
> + return NULL;
>   }
This will leak the 'env' python object. You probably also want to call
Py_DECREF here.

Cheers,

Jelmer


signature.asc
Description: This is a digitally signed message part

Re: [SCM] Samba Shared Repository - branch master updated

[Jelmer Vernooij]

On Tue, 2011-01-04 at 05:40 +0100, Andrew Bartlett wrote:
> commit 640028d158583825ea5ffd1266f099cf8776db5d
> Author: Andrew Bartlett 
> Date:   Tue Jan 4 14:41:38 2011 +1100
> 
> nsswitch Add talloc depencency for nsstest
> 
> nsstest does not use talloc, but it includes talloc.h via includes.h,
> and so without this it cannot find the right internal header.
> 
> This wasn't noticed before, as most linux developer boxes have a talloc.h.
> 
> The issue was reported by Adam Tauno Williams 
I'm a bit puzzled by this, as e.g. sn doesn't have talloc.h installed
either and neither have most of the build farm hosts. Perhaps it happens
if talloc.h is installed but not in /usr/include ?

Cheers,

Jelmer

Re: [SCM] Samba Shared Repository - branch master updated

[Jelmer Vernooij]

On Fri, 2010-12-10 at 22:35 +0100, Matthieu Patou wrote:
> commit 6875a611a71bf6d1623fc62361fc8b145b1f6559
> Author: Matthieu Patou 
> Date:   Fri Dec 10 23:16:28 2010 +0300
> 
> build: add a function to test if -lc is needed
> 
> This is needed on openbsd as some linking flags makes mandatory to
> specify the libc for the linking
> 
Urgh, there is something seriously broken if a C compiler doesn't link
to libc by default (I can understand a -nostdlib option). Is this
default OpenBSD behaviour or something specific to this host?

Cheers,

Jelmer

Re: [SCM] Samba Shared Repository - branch master updated

[Jelmer Vernooij]

Hi Matthieu,

On Sat, 2010-10-30 at 16:27 +0200, Matthieu Patou wrote:
> - Log -
> commit dce1a8f5233608d5bf59730200af3e041ce5
> Author: Matthieu Patou 
> Date:   Thu Oct 28 13:09:51 2010 +0400
> 
> provision: when deriving netbiosname from hostname force the netbiosname 
> to be compliant
> 
> It means no space/_/-/@ and less than 16 chars.
> 
> diff --git a/source4/scripting/python/samba/provision.py 
> b/source4/scripting/python/samba/provision.py
> index 5205ba5..49ad5d7 100644
> --- a/source4/scripting/python/samba/provision.py
> +++ b/source4/scripting/python/samba/provision.py
> @@ -443,6 +443,11 @@ def guess_names(lp=None, hostname=None, domain=None, 
> dnsdomain=None,
>  netbiosname = lp.get("netbios name")
>  if netbiosname is None:
>  netbiosname = hostname
> +# remove forbidden chars
> +for char in  " !#$%&'()-.@^_{}~":
> +netbiosname = "".join(netbiosname.split(char))
> +#force the length to be <16
> +netbiosname = netbiosname[0:15]
>  assert netbiosname is not None
>  netbiosname = netbiosname.upper()
>  if not valid_netbios_name(netbiosname):
> @@ -534,7 +539,14 @@ def make_smbconf(smbconf, setup_path, hostname, domain, 
> realm, serverrole,
>  assert smbconf is not None
>  if hostname is None:
>  hostname = socket.gethostname().split(".")[0]
> -netbiosname = hostname.upper()
> +netbiosname = hostname.upper()
> +# remove forbidden chars
> +for char in  " !#$%&'()-.@^_{}~":
> +netbiosname = "".join(netbiosname.split(char))
This logic is flawed - those characters are the ones that are allowed
(in addition to alphanumeric characters). Any other characters should be
stripped out. 

It would also be nice if we could share the list of acceptable NetBIOS
name characters between these two functions. 

> +#force the length to be <16
> +netbiosname = netbiosname[0:15]
If we have to do this sort of mangling I wonder if we shouldn't better
raise an error and let the user specify a name. 

Cheers,

Jelmer

Re: [SCM] Samba Shared Repository - branch master updated

[Jelmer Vernooij]

Hi Matthias,

On Fri, 2010-05-28 at 03:25 -0500, Matthias Dieter Wallnöfer wrote:
> diff --git a/source4/web_server/wsgi.c b/source4/web_server/wsgi.c
> index 7ee70e1..7a23ae4 100644
> --- a/source4/web_server/wsgi.c
> +++ b/source4/web_server/wsgi.c
> @@ -300,7 +300,9 @@ static PyObject *create_environ(bool tls, int 
> content_length, struct http_header
>   if (!strcasecmp(hdr->name, "Content-Type")) {
>   PyDict_SetItemString(env, "CONTENT_TYPE", 
> PyString_FromString(hdr->value));
>   } else { 
> - asprintf(&name, "HTTP_%s", hdr->name);
> + if (asprintf(&name, "HTTP_%s", hdr->name) < 0) {
> + continue;
> + }
>   PyDict_SetItemString(env, name, 
> PyString_FromString(hdr->value));
>   free(name);
>   }
^^^ Please don't ignore allocation problems, instead raise an exception
as I indicated in my review of your code in bugzilla.

Cheers,

Jelmer


signature.asc
Description: This is a digitally signed message part

Re: [SCM] Samba Shared Repository - branch master updated

[Jelmer Vernooij]

Hi Metze,

On Sun, 2010-08-08 at 04:08 -0500, Stefan Metzmacher wrote:
> - Log -
> commit ad94ae980462dc4c581a2fa1d7e927c2ae625c19
> Author: Stefan Metzmacher 
> Date:   Thu Aug 5 17:19:16 2010 +0200
> 
> librpc/idl/mgmt.idl: add missing size_is()
> 
> metze
> 
> commit 0a7f749bc80d9846b97cd22cd503473a205aaafd
> Author: Stefan Metzmacher 
> Date:   Thu Aug 5 16:10:37 2010 +0200
> 
> pidl:NDR: correctly handle bracket arrays with 'string'
> 
> metze
> 
> ---
> 
> Summary of changes:
>  librpc/idl/mgmt.idl|2 +-
>  pidl/lib/Parse/Pidl/NDR.pm |4 
>  2 files changed, 5 insertions(+), 1 deletions(-)
> 
> 
> Changeset truncated at 500 lines
> 
> diff --git a/librpc/idl/mgmt.idl b/librpc/idl/mgmt.idl
> index 35857f2..17c8cc4 100644
> --- a/librpc/idl/mgmt.idl
> +++ b/librpc/idl/mgmt.idl
> @@ -70,6 +70,6 @@ interface mgmt
>   WERROR mgmt_inq_princ_name (
>   [in]uint32  authn_proto,
>   [in]uint32  princ_name_size,
> - [out]   [string,charset(DOS)] uint8 princ_name[]
> + [out]   [string,size_is(princ_name_size),charset(DOS)] 
> uint8 princ_name[]
>   );
>  }
> diff --git a/pidl/lib/Parse/Pidl/NDR.pm b/pidl/lib/Parse/Pidl/NDR.pm
> index c1ac600..20f52bf 100644
> --- a/pidl/lib/Parse/Pidl/NDR.pm
> +++ b/pidl/lib/Parse/Pidl/NDR.pm
> @@ -125,6 +125,10 @@ sub GetElementLevelTable($$)
>   if ($d eq "*") {
>   $is_conformant = 1;
>   if ($size = shift @size_is) {
> + if (has_property($e, "string")) {
> + $is_string = 1;
> + delete($e->{PROPERTIES}->{string});
> + }
>   } elsif ((scalar(@size_is) == 0) and has_property($e, 
> "string")) {
>   $is_string = 1;
>   delete($e->{PROPERTIES}->{string});
This change appears to be breaking openchange:

http://buildbot.openchange.org/builders/kubuntu1004-server/builds/150/steps/compile/logs/stdio

gen_ndr/ndr_exchange.c: In function ‘ndr_push_StringsArray_r’:
gen_ndr/ndr_exchange.c:6530: error: expected expression before ‘;’ token
gen_ndr/ndr_exchange.c:6531: error: ‘_status’ undeclared (first use in
this function)
gen_ndr/ndr_exchange.c:6531: error: (Each undeclared identifier is
reported only once
gen_ndr/ndr_exchange.c:6531: error: for each function it appears in.)
gen_ndr/ndr_exchange.c:6536: error: expected expression before ‘;’ token

Cheers,

Jelmer

Re: [SCM] Samba Shared Repository - branch master updated

[Jelmer Vernooij]

Hi Matthias,

On Sat, 2010-04-17 at 13:12 -0500, Matthias Dieter Wallnöfer wrote:
> - Log -
> commit c678563efec82e84c9c86309cbf9495950a02b8c
> Author: Matthias Dieter Wallnöfer 
> Date:   Sat Apr 17 20:08:15 2010 +0200
> 
> s4:kdc/db-glue.c - use "TALLOC_FREE" insteal of "talloc_free" for the 
> "priv" context
> 
> Also after a free "priv" could be != NULL and may be freed again.
> This should fix bug #7365.
We tend to avoid TALLOC_FREE in Samba 4, opting for explicitly setting
variable to NULL explicitly. This way valgrind will tell us about double
frees. 

If you think we should consider using TALLOC_FREE, I think we should
discuss that first - for now, please use talloc_free() and assignments
to NULL.

Cheers,

Jelmer

Re: [SCM] Samba Shared Repository - branch master updated

[Björn JACKE]

On 2010-05-25 at 07:41 +0200 Stefan (metze) Metzmacher sent off:
> I think we also need -qhalt=e here, to match -Werror.
> 
> I'm wondering if --errwarn is the correct thing for the sun case...

-qhalt=w and -errwarn=%all are the equivalent options for -Werror. Some
configure checks need to bail out errors even on warnings, that's all what
this is needed for.

Cheers
Björn

Re: [SCM] Samba Shared Repository - branch master updated

[Björn JACKE]

Hi Metze,

On 2010-06-10 at 20:50 +0200 Stefan (metze) Metzmacher sent off:
> > +   *  h_errno is defined as function call macro for multithreaded 
> > applications
> > +   *  in HP-UX. *this* h_errno is not used in the HP-UX codepath of our nss
> > +   *  modules, so let's simply comment it out here:
> > +   * int h_errno;
> > +   */
> > nss_status_t status;
> >  } nss_XbyY_args_t;
> 
> Aren't this structures of fixed predefined size?
> Don't we use this to cast a given buffer into something we use?

you may very well be right. I didn't see this - the solaris/hpux shared nss
code is quite a pain to read


> If so args->status is wrong now and we better keep an 'int _not_used'.

alright, I'll change it this way, thanks!

Cheers
Björn
-- 
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-37-0, fax: +49-551-37-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen


pgpDGaPi6RKhX.pgp
Description: PGP signature

Re: [SCM] Samba Shared Repository - branch master updated

[simo]

On Sat, 2011-05-07 at 02:21 +0200, Jeremy Allison wrote:
> The branch, master has been updated
>via  ac25835 Fix Samba3 on OpenIndiana.
>   from  f7b3909 A couple more off-by-one calculations with strlcpy.
> 
> http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master
> 
> 
> - Log -
> commit ac25835ab7b76226bd59fec9ffef46d5c5817d54
> Author: Gordon Ross 
> Date:   Fri May 6 16:00:08 2011 -0700
> 
> Fix Samba3 on OpenIndiana.
> 
> I'd like Samba to use the native OpenLDAP and MIT Kerberos libs.
> Attached are some patches to do that. (relative to git master)
> It does not build for me without these.
> 
> (OpenIndiana is an off-shoot of OpenSolaris  See 
> http://www.openindiana.org)
> 
> Autobuild-User: Jeremy Allison 
> Autobuild-Date: Sat May  7 02:20:14 CEST 2011 on sn-devel-104
> 
> ---
> 
> Summary of changes:
>  auth/kerberos/gssapi_pac.c|   24 
>  lib/replace/system/kerberos.h |4 +++-
>  source3/configure.in  |   26 +-
>  source3/wscript   |2 +-
>  4 files changed, 49 insertions(+), 7 deletions(-)
> 
> 
> Changeset truncated at 500 lines:
> 
> diff --git a/auth/kerberos/gssapi_pac.c b/auth/kerberos/gssapi_pac.c
> index d89a649..e115cfe 100644
> --- a/auth/kerberos/gssapi_pac.c
> +++ b/auth/kerberos/gssapi_pac.c
> @@ -23,6 +23,30 @@
>  
>  #include "libcli/auth/krb5_wrap.h"
>  
> +#if 0
> +/* FIXME - need proper configure/waf test
> + * to determine if gss_mech_krb5 and friends
> + * exist. JRA.
> + */
> +/*
> + * These are not exported by Solaris -lkrb5
> + * Maybe move to libreplace somewhere?
> + */
> +static const gss_OID_desc krb5_gss_oid_array[] = {
> + /* this is the official, rfc-specified OID */
> + { 9, "\052\206\110\206\367\022\001\002\002" },
> + /* this is the pre-RFC mech OID */
> + { 5, "\053\005\001\005\002" },
> + /* this is the unofficial, incorrect mech OID emitted by MS */
> + { 9, "\052\206\110\202\367\022\001\002\002" },
> + { 0, 0 }
> +};

Jeremy something looks wrong here.
The comments seem to imply the MS and the official OID should be
different, (the MS being wrong), yet I see non difference between them.


Simo.

-- 
Simo Sorce
Samba Team GPL Compliance Officer 
Principal Software Engineer at Red Hat, Inc. 

Re: [SCM] Samba Shared Repository - branch master updated

[Jeremy Allison]

On Thu, Apr 21, 2011 at 07:14:02PM +0200, Günther Deschner wrote:
> The branch, master has been updated
>via  1b7cd33 s3-vfs: try to fix the Tru64 build.
>via  bb65187 s3-proto: move more rpc client prototypes to cli_pipe.h.
>   from  2b608f3 script: add hardcoded library release key id
> 
> http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master
> 
> 
> - Log -
> commit 1b7cd33291ec69f58a68396cf7406a6da3083df3
> Author: Günther Deschner 
> Date:   Thu Apr 21 18:26:40 2011 +0200
> 
> s3-vfs: try to fix the Tru64 build.
> 
> Volker, Jeremy please check.
> 

Shouldn't this be bracketed with an :

#ifdef vfsops
+change
#endif

Re: [SCM] Samba Shared Repository - branch master updated

[Stefan (metze) Metzmacher]

Hi Andrew,

> commit b32f155bae384bd3ae1d9ba9f246a0eca18a2511
> Author: Andrew Tridgell 
> Date:   Mon Mar 28 16:44:32 2011 +1100
> 
> s4-dsdb: perform FSMO transfers asynchronously
> 
> this gives the administrator a proper error message on the command
> line
> 
> Pair-Programmed-With: Andrew Bartlett 
> 
> commit 358892e8365d052d7b9300737a4c1dc92b26cc0c
> Author: Andrew Tridgell 
> Date:   Mon Mar 28 16:20:47 2011 +1100
> 
> s4-fsmo: make rootDSE modify for FSMO transfer async
> 
> this gives the ldap client the error code from the transfer
> 
> Pair-Programmed-With: Andrew Bartlett 

Not really related to your change, but I think we have a real problem
in this code, as rootdse_become_master() is called within a
transaction. Which may talks to another local process and then to a
remove server.

metze




signature.asc
Description: OpenPGP digital signature

Re: [SCM] Samba Shared Repository - branch master updated

[simo]

On Wed, 2011-03-09 at 09:04 +0100, Andrew Bartlett wrote:
> The branch, master has been updated
>via  9728b5a librpc/ndr use hyper for uid_t/gid_t rather than udlong
>   from  c4b52fb s3: Use talloc_tos() in idmap_nss_sids_to_unixids
> 
> http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master
> 
> 
> - Log -
> commit 9728b5a0d5a5c09615a09a97bf217bcacb773579
> Author: Andrew Bartlett 
> Date:   Wed Mar 2 19:06:15 2011 +1100
> 
> librpc/ndr use hyper for uid_t/gid_t rather than udlong
> 
> This has 8 byte alignment, which is what was specified in pidl for
> these types.

I am puzzled, uid_t and gid_t are 32 bit quantitiers on all platforms I
know, do you know of platforms that use 64 bit uid/gid types ?

If not, why do you use a 64 bit type for them ?

Also uid_t and gid_t are signed in many platforms while here you seem to
be casting to uint64_t, and that doesn't look right to me.

Care to comment ?

Simo.

-- 
Simo Sorce
Samba Team GPL Compliance Officer 
Principal Software Engineer at Red Hat, Inc. 

Re: [SCM] Samba Shared Repository - branch master updated

[Volker Lendecke]

On Tue, Mar 01, 2011 at 07:55:28AM -0500, simo wrote:
> If you want to see this patch in a slightly better form you can use a
> command like this:
> git showh -M -C --patience 5f5ca913b7abfcf95782339fac2dc8c1541b1126
> 
> With this one git is told to detect the rename and does show only the
> differences.

Ok, thanks. Then I rephrase my comment a bit. It would be
highly desirable to make small patches to both
implementations until there is no difference anymore or the
diff is really trivial. That patch is almost 2000 lines,
which is for my taste a LOT too much for a single patch
unless there is a very good reason for this.

Please in the future divide that kind of patches into much
smaller chunks if you do not want to actively feed
resistance against any kind of changes to Samba3.

With best regards,

Volker Lendecke

-- 
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-37-0, fax: +49-551-37-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen

Re: [SCM] Samba Shared Repository - branch master updated

[simo]

On Tue, 2011-03-01 at 07:35 +0100, Volker Lendecke wrote:
> On Tue, Mar 01, 2011 at 04:33:01AM +0100, Andrew Bartlett wrote:
> > The branch, master has been updated
> >via  5f5ca91 lib/util: new merged debug system
> >via  4acef31 lib/util move debug.[ch] out of the way
> >   from  b1f68b6 s4-libnet_vampire: Ignore some attributes when building 
> > working schema cache
> > 
> > http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master
> > 
> > 
> > - Log -
> > commit 5f5ca913b7abfcf95782339fac2dc8c1541b1126
> > Author: Andrew Bartlett 
> > Date:   Thu Feb 24 16:14:03 2011 +1100
> > 
> > lib/util: new merged debug system
> > 
> > This is the s3 debug system, with a number of changes to tidy it up
> > for common use.  The debug class system is simplified by the removal of 
> > the
> > ISSET table, the system no longer attempts to cope with assignment of
> > DEBUGLEVEL, and the full class table is always available (rather than
> > just DEBUGLEVEL_CLASS[DBCG_ALL]) from startup.  It is also no longer
> > confusingly described as a hack, but as the initial table.
> 
> The way git diff / git whatchanged works makes it more
> difficult than necessary to compare the actual code changes
> if you make changes and move the files at the same time.
> Next time, can you please split those two changes into two
> commits?

Volker,

If you want to see this patch in a slightly better form you can use a
command like this:
git showh -M -C --patience 5f5ca913b7abfcf95782339fac2dc8c1541b1126

With this one git is told to detect the rename and does show only the
differences.

you can use the same arguments for generating better patches when you
want to send them around for review, like this:
 git format-patch -M -C --patience --full-index -1
5f5ca913b7abfcf95782339fac2dc8c1541b1126

Simo.

-- 
Simo Sorce
Samba Team GPL Compliance Officer 
Principal Software Engineer at Red Hat, Inc. 

Re: [SCM] Samba Shared Repository - branch master updated

[Volker Lendecke]

On Tue, Mar 01, 2011 at 04:33:01AM +0100, Andrew Bartlett wrote:
> The branch, master has been updated
>via  5f5ca91 lib/util: new merged debug system
>via  4acef31 lib/util move debug.[ch] out of the way
>   from  b1f68b6 s4-libnet_vampire: Ignore some attributes when building 
> working schema cache
> 
> http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master
> 
> 
> - Log -
> commit 5f5ca913b7abfcf95782339fac2dc8c1541b1126
> Author: Andrew Bartlett 
> Date:   Thu Feb 24 16:14:03 2011 +1100
> 
> lib/util: new merged debug system
> 
> This is the s3 debug system, with a number of changes to tidy it up
> for common use.  The debug class system is simplified by the removal of 
> the
> ISSET table, the system no longer attempts to cope with assignment of
> DEBUGLEVEL, and the full class table is always available (rather than
> just DEBUGLEVEL_CLASS[DBCG_ALL]) from startup.  It is also no longer
> confusingly described as a hack, but as the initial table.

The way git diff / git whatchanged works makes it more
difficult than necessary to compare the actual code changes
if you make changes and move the files at the same time.
Next time, can you please split those two changes into two
commits?

Thanks,

Volker

-- 
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-37-0, fax: +49-551-37-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen

Re: [SCM] Samba Shared Repository - branch master updated

[Guenther Deschner]

On 02/28/2011 08:18 PM, Günther Deschner wrote:

The branch, master has been updated
via  26321c6 s3-torture: fix the build of rpc_open_tcp.
   from  eece80e s3-smbd: Pass tevent context to messaging functions.

http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master


- Log -
commit 26321c6010dbd461e85111daf358e6c23a0b47b2
Author: Günther Deschner
Date:   Mon Feb 28 20:18:33 2011 +0100

 s3-torture: fix the build of rpc_open_tcp.

 Guenther



Argl, this slipped to master w/o going through autobuild, sorry, was not 
meant to be like that :)


Guenther

--
Günther DeschnerGPG-ID: 8EE11688
Red Hat gdesch...@redhat.com
Samba Team  g...@samba.org

Re: [SCM] Samba Shared Repository - branch master updated

[Jelmer Vernooij]

On Fri, 2011-02-18 at 09:30 +0100, Andrew Bartlett wrote:
> - Log -
> commit 1ad8e5229f618fc04af371ba52b81f2e7e1f88f5
> Author: Andrew Bartlett 
> Date:   Fri Feb 18 13:59:05 2011 +1100
> 
> lib/util/charset Add back setlocale(), but only when called from binaries
> 
> When called from a library, we don't want to call this, as we may
> overwrite some of our calling program's context.
What in particular re-added the need for this? 

I'm worried about this change as it means we won't be testing the
library code that e.g. gets used as part of OpenChange and Evolution.

It would be really nice if we could get rid of our reliance on the
system locale for once and for all. :-/

Cheers,

Jelmer


> commit 2a3a86a86f3d1ab97adda563beda7ee35f6a2414
> Author: Andrew Bartlett 
> Date:   Fri Feb 18 13:47:28 2011 +1100
> 
> lib/util/charcnv Move iconv handle setup in common
> 
> We now use the struct smb_iconv_convenience at the core of all our
> iconv code, and use global_iconv_convenience for the callers that
> don't specify one.
> 
> Andrew Bartlett
> 
> commit 5155a5f5c130ff5b71ce4e37877378a6967046b4
> Author: Andrew Bartlett 
> Date:   Fri Feb 18 11:51:33 2011 +1100
> 
> s3-charcnv Don't genreate valid_table on the fly, rely on valid.dat
> 
> This file is always installed, and is only even required for the old,
> depricated mangle hash method.
> 
> Andrew Bartlett
> 
> commit e4c20230825dd78a2f06839dd7f6fe72f486a910
> Author: Andrew Bartlett 
> Date:   Fri Feb 18 10:52:12 2011 +1100
> 
> lib/util Remove #if _SAMBA_BUILD_ == 4 that isn't required any more
> 
> commit 8afc271e2a9950fda68e99928746623093cbd6ac
> Author: Andrew Bartlett 
> Date:   Fri Feb 18 10:24:58 2011 +1100
> 
> lib/util/charset Use top level iconv.c in source3
> 
> The two files were very similar already, the only change required was
> to adopt the s3 module registration fucntion name.
> 
> (NTSTATUS wasn't used as the charset code does not otherwise use that
> type).
> 
> Andrew Bartlett
> 
> Signed-off-by: Andrew Tridgell 
> 
> commit bed374215ff040cc995659d42938be35412a794a
> Author: Andrew Tridgell 
> Date:   Fri Feb 18 10:10:30 2011 +1100
> 
> s4-idl: rename s4 server_id.idl to server_id4.idl
> 
> this avoids a conflict with the new s3 server_id.idl
> 
> Pair-Programmed-With: Andrew Bartlett 
> 
> commit cfeceedfab64fe8b7f5823d1ada3f6d9b1799adf
> Author: Andrew Bartlett 
> Date:   Thu Feb 17 16:29:28 2011 +1100
> 
> librpc make ndr-standard a common library
> 
> commit 177ddcb084e8f8608bf2012a7c58f7b1e56d2acd
> Author: Andrew Bartlett 
> Date:   Thu Feb 17 16:19:45 2011 +1100
> 
> librpc push NDR_SECURITY and NDR_DCERPC to the top level wscript files
> 
> There isn't any reson to keep these private to source3 and source4 and
> more, and doing so creates problems for the top level build.
> 
> Andrew Bartlett
> 
> commit a493bf9103e5a36e0aef0d1bf1a055f602955d0f
> Author: Andrew Tridgell 
> Date:   Thu Feb 17 14:13:42 2011 +1100
> 
> nsswitch: depend on dl
> 
> The previous merge in this area didn't preserve the 'dl' depedency,
> which may be required on some platforms.
> 
> Pair-Programmed-With: Andrew Bartlett 
> 
> commit 92faeaeea841dd0bce9460d25429846ae2fdc0af
> Author: Andrew Bartlett 
> Date:   Wed Feb 16 16:30:56 2011 +1100
> 
> charset Remove use of {isupper,islower,toupper,tolower}_w functions
> 
> These now call the common _m functions that consider UTF16 code points.
> 
> This removes the code which will make up a 'lame' table in memory, as
> this can just as correctly be handled by running the algorithm at runtime 
> (which is to call toupper() and tolower() on characters < 128).
> 
> When used, a top level waf build will always locate the correct table
> - in the build tree or outside - due to relinking the installed
> binary.
> 
> Andrew Bartlett
> 
> commit 0bad0e3ff2063f009557ab6ad7a442ceaed593ee
> Author: Andrew Bartlett 
> Date:   Tue Feb 15 16:34:02 2011 +1100
> 
> s3-libads Remove MIT-specific krb5_princ_realm macro calls.
> 
> When compiled against heimdal, we need to use a more elegant API.
> 
> Andrew Bartlett
> 
> commit e4c2f5c474b2c294341edcce8044319a111e1e45
> Author: Andrew Bartlett 
> Date:   Fri Feb 18 16:04:30 2011 +1100
> 
> s3-waf use lib/util/wscript_build, and avoid duplicate subsystems
> 
> commit 83a63baecc0b49dac5547551436f9fca2595f2f7
> Author: Andrew Bartlett 
> Date:   Thu Feb 17 16:39:07 2011 +1100
> 
> lib/util Make UTIL_LDB conditional on an s4 build
> 
> This allows this file to be used in common.
> 
> commit c8b2b109760b366bee6dcf7c6c306936d271f508
> Author: Andrew Tridgell 
> Date:   Wed Feb 9 15:56:20 2011 +1100
> 
> s3-waf: use SAMBA3_*() build rules in source3/bu

Re: [SCM] Samba Shared Repository - branch master updated

[Jeremy Allison]

On Wed, Jan 19, 2011 at 11:11:01PM +0100, Andreas Schneider wrote:
> 
> commit 7a97518e12054e1f84236501322b756c5c09d7dd
> Author: Andreas Schneider 
> Date:   Wed Jan 19 16:22:28 2011 +0100
> 
> s3-smbd: Fixed a possible null pointer dereference.
> 
> index dd6af6d..b573a6c 100644
> --- a/source3/smbd/uid.c
> +++ b/source3/smbd/uid.c
> @@ -229,7 +229,7 @@ void conn_clear_vuid_cache(connection_struct *conn, 
> uint16_t vuid)
>  bool change_to_user(connection_struct *conn, uint16 vuid)
>  {
>   const struct auth_serversupplied_info *server_info = NULL;
> - user_struct *vuser = get_valid_user_struct(conn->sconn, vuid);
> + user_struct *vuser;
>   int snum;
>   gid_t gid;
>   uid_t uid;
> @@ -242,6 +242,8 @@ bool change_to_user(connection_struct *conn, uint16 vuid)
>   return(False);
>   }
>  
> + vuser = get_valid_user_struct(conn->sconn, vuid);
> +
>   /*
>* We need a separate check in security=share mode due to vuid
>* always being UID_FIELD_INVALID. If we don't do this then

Andreas, can you please explain this commit *exactly*.

How does leaving vuser undefined fix this ?

Jeremy.

Re: [SCM] Samba Shared Repository - branch master updated

[Jelmer Vernooij]

On Sun, 2010-12-12 at 22:50 +0100, Matthias Dieter Wallnöfer wrote:
> Hi Jelmer,
> 
> Jelmer Vernooij wrote:
> > The same goes for that macro - if it doesn't deal with proper free'ing,
> > then why not avoid it rather than rewrite the rest of the function that
> > uses it?
> >
> > discard_const_p is bad, and we should avoid it unless we really can.
> >
> well, I agree that "discard_const_p" isn't so nice. But I think the 
> right solution (if we make use of memory contexts) would be to never 
> derive them from NULL, but from a "self" instance or kind of this or if 
> not possible (static) the LDB context.
Even if we do that then it should be as a safeguard, not as an excuse
not to call talloc_free(). 

The advantage of hanging them off NULL is that they can more easily be
found using valgrind.

If we don't free these kinds of contexts explicitly and they are
allocated as children of e.g. the ldb context then that means they will
stay around for as long as the ldb context stays around, which can be a
very long time. So in some cases this will help contain the effect of
the leak but it's by no means an alternative.

Cheers,

Jelmer


signature.asc
Description: This is a digitally signed message part

Re: [SCM] Samba Shared Repository - branch master updated

[Matthias Dieter Wallnöfer]

Hi Jelmer,

Jelmer Vernooij wrote:

The same goes for that macro - if it doesn't deal with proper free'ing,
then why not avoid it rather than rewrite the rest of the function that
uses it?

discard_const_p is bad, and we should avoid it unless we really can.
   
well, I agree that "discard_const_p" isn't so nice. But I think the 
right solution (if we make use of memory contexts) would be to never 
derive them from NULL, but from a "self" instance or kind of this or if 
not possible (static) the LDB context.
   

diff --git a/source4/lib/ldb/pyldb_util.c b/source4/lib/ldb/pyldb_util.c
index 3e015d0..35071f3 100644
--- a/source4/lib/ldb/pyldb_util.c
+++ b/source4/lib/ldb/pyldb_util.c
@@ -23,10 +23,7 @@
   License along with this library; if not, 
see.
*/

-#include
-#include "replace.h"
#include "pyldb.h"
-#include

static PyObject *ldb_module = NULL;


 

See above. Python.h is included for a reason. Also, replace.h might not
be necessary on your system but necessary on others (as some
functionality is not provided by the OS).

   

But let me think - we include system headers only by libreplace. So at
the end it doesn't matter if replace or a system header defines it.
Obviously there we have no need for a system call - otherwise the build
would have broken.

 

replace.h isn't the only way in which we get system headers, e.g.
Python.h also includes a bunch - at least stdlib.h, unistd.h, stddef.h,
string.h, stdio.h, limits.h and assert.h.

   

In this special case we don't need the "replace.h" anymore since I do
now include "ldb_private.h" (which itself includes it).
 

Is there any particular reason why pyldb_util.c requires ldb_private.h ?
We should avoid including it if we can (and thus avoid tying pyldb_util
to a specific version of ldb).
   

Okay.

PS: Could you please start the implementation of the other message
attribute-mapping function in pyldb?
 

Sorry, that's on my todo list as are several other things. I'd be happy
to review a patch that adds it though.
   

Okay.

Cheers,
Matthias

Re: [SCM] Samba Shared Repository - branch master updated

[Jelmer Vernooij]

Hi Matthias,

On Sun, 2010-12-12 at 22:22 +0100, Matthias Dieter Wallnöfer wrote:
> Jelmer Vernooij wrote:
> > On Sun, 2010-12-12 at 21:29 +0100, Matthias Dieter Wallnöfer wrote:
> >
> >> Jelmer Vernooij wrote:
> >>  
> >>> On Sun, 2010-12-12 at 20:51 +0100, Matthias Dieter Wallnöfer wrote:
> >>>
>  @@ -248,27 +253,19 @@ static PyObject 
>  *py_dsdb_get_oid_from_attid(PyObject *self, PyObject *args)
> 
>   PyErr_LDB_OR_RAISE(py_ldb, ldb);
> 
>  -mem_ctx = talloc_new(NULL);
>  -if (mem_ctx == NULL) {
>  -   PyErr_NoMemory();
>  -   return NULL;
>  -}
>  -
>   schema = dsdb_get_schema(ldb, NULL);
> 
>   if (!schema) {
>   PyErr_SetString(PyExc_RuntimeError, "Failed to find a 
>  schema from ldb \n");
>  -talloc_free(mem_ctx);
>   return NULL;
>   }
>   
>   status = dsdb_schema_pfm_oid_from_attid(schema->prefixmap, 
>  attid,
>  -mem_ctx,&oid);
>  +NULL,&oid);
>   PyErr_WERROR_IS_ERR_RAISE(status);
> 
>   ret = PyString_FromString(oid);
>  -
>  -talloc_free(mem_ctx);
>  +talloc_free(discard_const_p(char, oid));
> 
>   
> >>> ^^ Is this really necessary? I'd rather have the extra memory context
> >>> than add an extra discard_const_p.
> >>>
> >>>
> >> I've really thought hard about this change - but it seems more correct
> >> to me.
> >> The problem is that the memory context isn't freed when
> >> PyERR_WERROR_IS_ERR_RAISE raises an exception.
> >>  
> > I suspect you mean PyErr_LDB_OR_RAISE?
> >
> > That macro never does a return at the moment, it's just a stub for "ldb
> > = PyLdb_AsLdbContext". Even if it did return, I think we should just
> > avoid using it, and manually check whether py_ldb is a ldb handle and
> > talloc_free and return if it isn't.
> >
> No Jelmer, I mean the macro below "dsdb_schema_pfm_oid_from_attid". When 
> that one raises an exception, what does succeed to "tmp_ctx"? I imagine 
> that it never will be freed - therefore I've provided this fix.
The same goes for that macro - if it doesn't deal with proper free'ing,
then why not avoid it rather than rewrite the rest of the function that
uses it?

discard_const_p is bad, and we should avoid it unless we really can.

>  diff --git a/source4/lib/ldb/pyldb_util.c b/source4/lib/ldb/pyldb_util.c
>  index 3e015d0..35071f3 100644
>  --- a/source4/lib/ldb/pyldb_util.c
>  +++ b/source4/lib/ldb/pyldb_util.c
>  @@ -23,10 +23,7 @@
>    License along with this library; if not, 
>  see.
> */
> 
>  -#include
>  -#include "replace.h"
> #include "pyldb.h"
>  -#include
> 
> static PyObject *ldb_module = NULL;
> 
>   
> >>> See above. Python.h is included for a reason. Also, replace.h might not
> >>> be necessary on your system but necessary on others (as some
> >>> functionality is not provided by the OS).
> >>>
> >> But let me think - we include system headers only by libreplace. So at
> >> the end it doesn't matter if replace or a system header defines it.
> >> Obviously there we have no need for a system call - otherwise the build
> >> would have broken.
> >>  
> > replace.h isn't the only way in which we get system headers, e.g.
> > Python.h also includes a bunch - at least stdlib.h, unistd.h, stddef.h,
> > string.h, stdio.h, limits.h and assert.h.
> >
> In this special case we don't need the "replace.h" anymore since I do 
> now include "ldb_private.h" (which itself includes it).
Is there any particular reason why pyldb_util.c requires ldb_private.h ?
We should avoid including it if we can (and thus avoid tying pyldb_util
to a specific version of ldb).

> PS: Could you please start the implementation of the other message 
> attribute-mapping function in pyldb?
Sorry, that's on my todo list as are several other things. I'd be happy
to review a patch that adds it though.

Cheers,

Jelmer


signature.asc
Description: This is a digitally signed message part

Re: [SCM] Samba Shared Repository - branch master updated

[Matthias Dieter Wallnöfer]

Hi Jelmer,

Jelmer Vernooij wrote:

On Sun, 2010-12-12 at 21:29 +0100, Matthias Dieter Wallnöfer wrote:
   

Jelmer Vernooij wrote:
 

On Sun, 2010-12-12 at 20:51 +0100, Matthias Dieter Wallnöfer wrote:
   

@@ -248,27 +253,19 @@ static PyObject *py_dsdb_get_oid_from_attid(PyObject 
*self, PyObject *args)

PyErr_LDB_OR_RAISE(py_ldb, ldb);

-   mem_ctx = talloc_new(NULL);
-   if (mem_ctx == NULL) {
-  PyErr_NoMemory();
-  return NULL;
-   }
-
schema = dsdb_get_schema(ldb, NULL);

if (!schema) {
PyErr_SetString(PyExc_RuntimeError, "Failed to find a schema from 
ldb \n");
-   talloc_free(mem_ctx);
return NULL;
}

status = dsdb_schema_pfm_oid_from_attid(schema->prefixmap, attid,
-   mem_ctx,&oid);
+   NULL,&oid);
PyErr_WERROR_IS_ERR_RAISE(status);

ret = PyString_FromString(oid);
-
-   talloc_free(mem_ctx);
+   talloc_free(discard_const_p(char, oid));

 

^^ Is this really necessary? I'd rather have the extra memory context
than add an extra discard_const_p.

   

I've really thought hard about this change - but it seems more correct
to me.
The problem is that the memory context isn't freed when
PyERR_WERROR_IS_ERR_RAISE raises an exception.
 

I suspect you mean PyErr_LDB_OR_RAISE?

That macro never does a return at the moment, it's just a stub for "ldb
= PyLdb_AsLdbContext". Even if it did return, I think we should just
avoid using it, and manually check whether py_ldb is a ldb handle and
talloc_free and return if it isn't.
   
No Jelmer, I mean the macro below "dsdb_schema_pfm_oid_from_attid". When 
that one raises an exception, what does succeed to "tmp_ctx"? I imagine 
that it never will be freed - therefore I've provided this fix.
   

diff --git a/source4/lib/ldb-samba/pyldb.c b/source4/lib/ldb-samba/pyldb.c
index e8cdb90..f198d74 100644
--- a/source4/lib/ldb-samba/pyldb.c
+++ b/source4/lib/ldb-samba/pyldb.c
@@ -19,10 +19,8 @@
  License along with this library; if not, 
see.
   */

-#include
-#include "includes.h"
-#include
   #include "lib/ldb/pyldb.h"
+#include "includes.h"
   #include "param/pyparam.h"
   #include "auth/credentials/pycredentials.h"
   #include "ldb_wrap.h"

 

Can you please stop reordering include files? There's a good reason
Python.h is included first, it prevents warnings on some systems.

What is the benefit of this sort of reordering?

   

"pyldb.h" includes"" - therefore I had to do the reordering.
 

But why did you have to change these include lines at all? I can
understand removing #include lines that are not necessary but there's no
need to use the smallest subset of include lines. If an include file was
already processed earlier then the overhead of including it again is
minimal.
   
Sorry, this was a mistake - the previous behaviour was better. I will 
push a revert fix.
   

diff --git a/source4/lib/ldb/pyldb_util.c b/source4/lib/ldb/pyldb_util.c
index 3e015d0..35071f3 100644
--- a/source4/lib/ldb/pyldb_util.c
+++ b/source4/lib/ldb/pyldb_util.c
@@ -23,10 +23,7 @@
  License along with this library; if not, 
see.
   */

-#include
-#include "replace.h"
   #include "pyldb.h"
-#include

   static PyObject *ldb_module = NULL;

 

See above. Python.h is included for a reason. Also, replace.h might not
be necessary on your system but necessary on others (as some
functionality is not provided by the OS).
   

But let me think - we include system headers only by libreplace. So at
the end it doesn't matter if replace or a system header defines it.
Obviously there we have no need for a system call - otherwise the build
would have broken.
 

replace.h isn't the only way in which we get system headers, e.g.
Python.h also includes a bunch - at least stdlib.h, unistd.h, stddef.h,
string.h, stdio.h, limits.h and assert.h.
   
In this special case we don't need the "replace.h" anymore since I do 
now include "ldb_private.h" (which itself includes it).
   

diff --git a/source4/libnet/py_net.c b/source4/libnet/py_net.c
index 9775e24..28dee59 100644
--- a/source4/libnet/py_net.c
+++ b/source4/libnet/py_net.c
@@ -18,19 +18,15 @@
  along with this program.  If not, see.
   */

-#include
+#include "lib/ldb/pyldb.h"

 

^^ We shouldn't include lib/ldb/pyldb.h directly, but always
in case the system pyldb is being used.
   

Could you point out a rule when to use "lib/ldb/pyldb.h" or?
This isn't clear.
 

Always use, except in lib/ldb/ itself.
   

Okay, I will fix this.

PS: Could you please start the implementation of the other message 
attribute-mapping function in pyldb?


Cheers,
Matthias

Re: [SCM] Samba Shared Repository - branch master updated

[Jelmer Vernooij]

On Sun, 2010-12-12 at 21:29 +0100, Matthias Dieter Wallnöfer wrote:
> Jelmer Vernooij wrote:
> > On Sun, 2010-12-12 at 20:51 +0100, Matthias Dieter Wallnöfer wrote:
> >> @@ -248,27 +253,19 @@ static PyObject *py_dsdb_get_oid_from_attid(PyObject 
> >> *self, PyObject *args)
> >>
> >>PyErr_LDB_OR_RAISE(py_ldb, ldb);
> >>
> >> -  mem_ctx = talloc_new(NULL);
> >> -  if (mem_ctx == NULL) {
> >> - PyErr_NoMemory();
> >> - return NULL;
> >> -  }
> >> -
> >>schema = dsdb_get_schema(ldb, NULL);
> >>
> >>if (!schema) {
> >>PyErr_SetString(PyExc_RuntimeError, "Failed to find a schema 
> >> from ldb \n");
> >> -  talloc_free(mem_ctx);
> >>return NULL;
> >>}
> >>
> >>status = dsdb_schema_pfm_oid_from_attid(schema->prefixmap, attid,
> >> -  mem_ctx,&oid);
> >> +  NULL,&oid);
> >>PyErr_WERROR_IS_ERR_RAISE(status);
> >>
> >>ret = PyString_FromString(oid);
> >> -
> >> -  talloc_free(mem_ctx);
> >> +  talloc_free(discard_const_p(char, oid));
> >>  
> > ^^ Is this really necessary? I'd rather have the extra memory context
> > than add an extra discard_const_p.
> >
> I've really thought hard about this change - but it seems more correct 
> to me.
> The problem is that the memory context isn't freed when 
> PyERR_WERROR_IS_ERR_RAISE raises an exception.
I suspect you mean PyErr_LDB_OR_RAISE?

That macro never does a return at the moment, it's just a stub for "ldb
= PyLdb_AsLdbContext". Even if it did return, I think we should just
avoid using it, and manually check whether py_ldb is a ldb handle and
talloc_free and return if it isn't. 

> >> diff --git a/source4/lib/ldb-samba/pyldb.c b/source4/lib/ldb-samba/pyldb.c
> >> index e8cdb90..f198d74 100644
> >> --- a/source4/lib/ldb-samba/pyldb.c
> >> +++ b/source4/lib/ldb-samba/pyldb.c
> >> @@ -19,10 +19,8 @@
> >>  License along with this library; if not, 
> >> see.
> >>   */
> >>
> >> -#include
> >> -#include "includes.h"
> >> -#include
> >>   #include "lib/ldb/pyldb.h"
> >> +#include "includes.h"
> >>   #include "param/pyparam.h"
> >>   #include "auth/credentials/pycredentials.h"
> >>   #include "ldb_wrap.h"
> >>  
> > Can you please stop reordering include files? There's a good reason
> > Python.h is included first, it prevents warnings on some systems.
> >
> > What is the benefit of this sort of reordering?
> >
> "pyldb.h" includes "" - therefore I had to do the reordering.
But why did you have to change these include lines at all? I can
understand removing #include lines that are not necessary but there's no
need to use the smallest subset of include lines. If an include file was
already processed earlier then the overhead of including it again is
minimal.

> >> diff --git a/source4/lib/ldb/pyldb_util.c b/source4/lib/ldb/pyldb_util.c
> >> index 3e015d0..35071f3 100644
> >> --- a/source4/lib/ldb/pyldb_util.c
> >> +++ b/source4/lib/ldb/pyldb_util.c
> >> @@ -23,10 +23,7 @@
> >>  License along with this library; if not, 
> >> see.
> >>   */
> >>
> >> -#include
> >> -#include "replace.h"
> >>   #include "pyldb.h"
> >> -#include
> >>
> >>   static PyObject *ldb_module = NULL;
> >>  
> > See above. Python.h is included for a reason. Also, replace.h might not
> > be necessary on your system but necessary on others (as some
> > functionality is not provided by the OS).
> But let me think - we include system headers only by libreplace. So at 
> the end it doesn't matter if replace or a system header defines it. 
> Obviously there we have no need for a system call - otherwise the build 
> would have broken.
replace.h isn't the only way in which we get system headers, e.g.
Python.h also includes a bunch - at least stdlib.h, unistd.h, stddef.h,
string.h, stdio.h, limits.h and assert.h.

> >> diff --git a/source4/libnet/py_net.c b/source4/libnet/py_net.c
> >> index 9775e24..28dee59 100644
> >> --- a/source4/libnet/py_net.c
> >> +++ b/source4/libnet/py_net.c
> >> @@ -18,19 +18,15 @@
> >>  along with this program.  If not, see.
> >>   */
> >>
> >> -#include
> >> +#include "lib/ldb/pyldb.h"
> >>  
> > ^^ We shouldn't include lib/ldb/pyldb.h directly, but always
> > in case the system pyldb is being used.
> Could you point out a rule when to use "lib/ldb/pyldb.h" or ? 
> This isn't clear.
Always use , except in lib/ldb/ itself.

Cheers,

Jelmer


signature.asc
Description: This is a digitally signed message part

Re: [SCM] Samba Shared Repository - branch master updated

[Matthias Dieter Wallnöfer]

Hi Jelmer,

Jelmer Vernooij wrote:

On Sun, 2010-12-12 at 20:51 +0100, Matthias Dieter Wallnöfer wrote:
   

@@ -163,6 +165,8 @@ static PyObject *py_samdb_set_ntds_settings_dn(PyLdbObject 
*self, PyObject *args
}

if (!PyObject_AsDn(tmp_ctx, py_ntds_settings_dn, 
ldb,&ntds_settings_dn)) {
+   PyErr_NoMemory();
+   talloc_free(tmp_ctx);
return NULL;
}
 

This is incorrect, PyObject_AsDn will already set an exception itself.
The fact that it fails is not necessarily an indication of an out of
memory error.
   

ah - sorry.

@@ -248,27 +253,19 @@ static PyObject *py_dsdb_get_oid_from_attid(PyObject 
*self, PyObject *args)

PyErr_LDB_OR_RAISE(py_ldb, ldb);

-   mem_ctx = talloc_new(NULL);
-   if (mem_ctx == NULL) {
-  PyErr_NoMemory();
-  return NULL;
-   }
-
schema = dsdb_get_schema(ldb, NULL);

if (!schema) {
PyErr_SetString(PyExc_RuntimeError, "Failed to find a schema from 
ldb \n");
-   talloc_free(mem_ctx);
return NULL;
}

status = dsdb_schema_pfm_oid_from_attid(schema->prefixmap, attid,
-   mem_ctx,&oid);
+   NULL,&oid);
PyErr_WERROR_IS_ERR_RAISE(status);

ret = PyString_FromString(oid);
-
-   talloc_free(mem_ctx);
+   talloc_free(discard_const_p(char, oid));
 

^^ Is this really necessary? I'd rather have the extra memory context
than add an extra discard_const_p.
   
I've really thought hard about this change - but it seems more correct 
to me.
The problem is that the memory context isn't freed when 
PyERR_WERROR_IS_ERR_RAISE raises an exception.

diff --git a/source4/lib/ldb-samba/pyldb.c b/source4/lib/ldb-samba/pyldb.c
index e8cdb90..f198d74 100644
--- a/source4/lib/ldb-samba/pyldb.c
+++ b/source4/lib/ldb-samba/pyldb.c
@@ -19,10 +19,8 @@
 License along with this library; if not, see.
  */

-#include
-#include "includes.h"
-#include
  #include "lib/ldb/pyldb.h"
+#include "includes.h"
  #include "param/pyparam.h"
  #include "auth/credentials/pycredentials.h"
  #include "ldb_wrap.h"
 

Can you please stop reordering include files? There's a good reason
Python.h is included first, it prevents warnings on some systems.

What is the benefit of this sort of reordering?
   

"pyldb.h" includes "" - therefore I had to do the reordering.

diff --git a/source4/lib/ldb/pyldb.c b/source4/lib/ldb/pyldb.c
index 3bee9ab..44a006f 100644
--- a/source4/lib/ldb/pyldb.c
+++ b/source4/lib/ldb/pyldb.c
@@ -26,9 +26,6 @@
 License along with this library; if not, see.
  */

-#include
-#include "replace.h"
-#include "ldb_private.h"
  #include "pyldb.h"

  /* There's no Py_ssize_t in 2.4, apparently */
 

Same here.

   

diff --git a/source4/lib/ldb/pyldb.h b/source4/lib/ldb/pyldb.h
index 1f4bdf7..afc8c51 100644
--- a/source4/lib/ldb/pyldb.h
+++ b/source4/lib/ldb/pyldb.h
@@ -28,6 +28,7 @@

  #include
  #include
+#include "ldb_private.h"

  typedef struct {
PyObject_HEAD
 

^^^ We can't include ldb_private.h here, it's not installed so this will
break system installs of pyldb.
   

Oh sorry - that wasn't clear. I will change this.

diff --git a/source4/lib/ldb/pyldb_util.c b/source4/lib/ldb/pyldb_util.c
index 3e015d0..35071f3 100644
--- a/source4/lib/ldb/pyldb_util.c
+++ b/source4/lib/ldb/pyldb_util.c
@@ -23,10 +23,7 @@
 License along with this library; if not, see.
  */

-#include
-#include "replace.h"
  #include "pyldb.h"
-#include

  static PyObject *ldb_module = NULL;
 

See above. Python.h is included for a reason. Also, replace.h might not
be necessary on your system but necessary on others (as some
functionality is not provided by the OS).
   
But let me think - we include system headers only by libreplace. So at 
the end it doesn't matter if replace or a system header defines it. 
Obviously there we have no need for a system call - otherwise the build 
would have broken.
   

diff --git a/source4/libnet/py_net.c b/source4/libnet/py_net.c
index 9775e24..28dee59 100644
--- a/source4/libnet/py_net.c
+++ b/source4/libnet/py_net.c
@@ -18,19 +18,15 @@
 along with this program.  If not, see.
  */

-#include
+#include "lib/ldb/pyldb.h"
 

^^ We shouldn't include lib/ldb/pyldb.h directly, but always
in case the system pyldb is being used.
   
Could you point out a rule when to use "lib/ldb/pyldb.h" or ? 
This isn't clear.


Cheers,
Matthias