WERR_OK vs WERR_SUCCESS (was: Re: [PATCH] add generated NTSTATUS error module for python)
Andrew Bartlett
abartlet at samba.org
Sun Feb 12 03:11:08 UTC 2017
On Sun, 2017-02-12 at 09:13 +1300, Andrew Bartlett wrote:
> On Wed, 2017-02-01 at 15:15 +1300, Bob Campbell wrote:
> > Hi again,
> >
> > Attached are some patches that build on top of the NTSTATUS patches
> > to
> > also generate WERROR codes and descriptions, as well as a similar
> > script
> > to the one attached with the NTSTATUS patches to make sure that we
> > aren't missing any.
> >
> > Please review and push if appropriate, once the NTSTATUS patches
> > (currently in autobuild) have landed.
>
> Sadly this fails autobuild, so I did a bisect with:
>
> ./script/bisect-test.py --good=origin/master --bad=gen-error --test-
> command='make test FAIL_IMMEDIATELY=1 TESTS=acl.py'
>
> I've pushed the gen-error branch I've used to my repo.
>
> 3c24ff1d189b7eade045591975d361d13e59b6c6 is the first bad commit
> commit 3c24ff1d189b7eade045591975d361d13e59b6c6
> Author: Bob Campbell <bobcampbell at catalyst.net.nz>
> Date: Fri Jan 20 12:24:53 2017 +1300
>
> errors: add WERROR generation to build system
>
> Parts of doserr.c and werror.h are now generated into
> werror_gen.c
> and
> werror_gen.h, respectively. Also, py_werror.c is now generated.
>
> Some errors were not included in the list which we now generate
> WERRORs
> from. These errors have been manually included.
>
> Signed-off-by: Bob Campbell <bobcampbell at catalyst.net.nz>
> Reviewed-by: Andrew Bartlett <abartlet at samba.org>
> Reviewed-by: Garming Sam <garming at catalyst.net.nz>
>
> Fails with:
>
> UNEXPECTED(error):
> samba4.ldap.acl.python(ad_dc_ntvfs).__main__.AclSPNTests.test_compute
> r_
> spn(ad_dc_ntvfs)
> REASON: Exception: Exception: Traceback (most recent call last):
> File "/data/samba/git/samba/source4/dsdb/tests/python/acl.py", line
> 1792, in setUp
> self.create_dc(self.dcctx)
> File "/data/samba/git/samba/source4/dsdb/tests/python/acl.py", line
> 1870, in create_dc
> ctx.join_add_objects()
> File "bin/python/samba/join.py", line 614, in join_add_objects
> ctx.join_add_ntdsdsa()
> File "bin/python/samba/join.py", line 545, in join_add_ntdsdsa
> ctx.DsAddEntry([rec])
> File "bin/python/samba/join.py", line 495, in DsAddEntry
> ctr.err_data.info.extended_err))
> AttributeError: 'NoneType' object has no attribute 'extended_err'
>
>
> Can you please look at this?
This was bugging me, so I looked into what we need to do:
The issue is that this change changed the string from of 0 from WERR_OK
to WERR_SUCCESS. You tried to cater to that by leaving WERR_OK in the
table in doserr.c, but the generated table applies first.
We need to ensure that entries in the combined table are unique, that
the final table matches the original one (other than odering), and have
a pre-commit that renames WERR_OK to WERR_SUCCESS.
Then we need to get out of the game of using strings and embedded
constants in the python code. Here is the list that needs to be fixed:
python/samba/join.py: if ctr.extended_err != (0, 'WERR_OK'):
python/samba/join.py: if ctr.err_data.status != (0,
'WERR_OK'):
python/samba/netcmd/dns.py: if e.args[1] ==
'WERR_DNS_ERROR_NAME_DOES_NOT_EXIST':
python/samba/netcmd/dns.py: if e.args[1] ==
'WERR_DNS_ERROR_ZONE_ALREADY_EXISTS':
python/samba/netcmd/dns.py: if e.args[1] ==
'WERR_DNS_ERROR_ZONE_DOES_NOT_EXIST':
python/samba/netcmd/dns.py: if e.args[1] ==
'WERR_DNS_ERROR_NAME_DOES_NOT_EXIST':
python/samba/netcmd/dns.py: if e.args[1] ==
'WERR_DNS_ERROR_NAME_DOES_NOT_EXIST':
python/samba/netcmd/dns.py: if e.args[1] ==
'WERR_DNS_ERROR_NAME_DOES_NOT_EXIST':
python/samba/netcmd/dns.py: if e.args[1] ==
'WERR_DNS_ERROR_NAME_DOES_NOT_EXIST':
python/samba/netcmd/domain.py: if werr == 8452:
#WERR_DS_DRA_NO_REPLICA
python/samba/netcmd/domain.py: if werr == 8452:
#WERR_DS_DRA_NO_REPLICA
python/samba/netcmd/domain.py: WERR_OK = 0x00000000
python/samba/netcmd/domain.py: WERR_INVALID_FUNCTION = 0x00000001
python/samba/netcmd/domain.py: WERR_NERR_ACFNOTLOADED = 0x000008B3
python/samba/netcmd/domain.py: WERR_RPC_S_PROCNUM_OUT_OF_RANGE =
0x000006D1
python/samba/netcmd/domain.py: if
self.check_runtime_error(error, self.WERR_RPC_S_PROCNUM_OUT_OF_RANGE):
python/samba/netcmd/domain.py: if local_trust_status !=
self.WERR_OK or local_conn_status != self.WERR_OK:
python/samba/netcmd/domain.py: if
remote_trust_status != self.WERR_OK or remote_conn_status !=
self.WERR_OK:
python/samba/netcmd/domain.py: if local_trust_status !=
self.WERR_OK or local_conn_status != self.WERR_OK:
python/samba/netcmd/domain.py: if local_conn_status !=
self.WERR_OK:
python/samba/netcmd/domain.py: if remote_trust_status !=
self.WERR_OK or remote_conn_status != self.WERR_OK:
python/samba/netcmd/domain.py: if remote_conn_status !=
self.WERR_OK:
python/samba/netcmd/domain.py: if
self.check_runtime_error(error, self.WERR_RPC_S_PROCNUM_OUT_OF_RANGE):
python/samba/netcmd/domain.py: if
self.check_runtime_error(error, self.WERR_INVALID_FUNCTION):
python/samba/netcmd/domain.py: if
self.check_runtime_error(error, self.WERR_NERR_ACFNOTLOADED):
python/samba/remove_dc.py: if enum == 0x000025F2:
#WERR_DNS_ERROR_NAME_DOES_NOT_EXIST
python/samba/remove_dc.py: if enum == 0x000025F2:
#WERR_DNS_ERROR_NAME_DOES_NOT_EXIST
python/samba/tests/dns.py: if num != 9601:
#WERR_DNS_ERROR_ZONE_DOES_NOT_EXIST
I realise this is a pile more work, this technical debt has built up
over quite some time. Thankfully others have been working on the
problem, (see the commits by Günther and Kamen in the doserr.c
history).
I realise WERR_SUCCESS is not a succinct but thankfully we don't use
windows error codes as often, so I think WERR_OK it can go the way of
NT_STATUS_NOPROBLEMO, but if there are objections we can keep it as a C
-only #define.
Thanks,
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
More information about the samba-technical
mailing list