[PATCH] add generated NTSTATUS error module for python

Bob Campbell bobcampbell at catalyst.net.nz
Thu Jan 12 23:45:33 UTC 2017


Hi all,

Here are some patches, including Guenther's original patch, to move
ntstatus generation to generated code.

One of the purposes of these patches is to separate generated from
non-generated files. Previously, we had partially generated files, which
meant that we didn't generate them during the build. This makes it a lot
cleaner and easier to update.

My first patch fixes some complaints with the initial patch -
specifically, Andrew and Metze's comments.

The second one allows gen_ntstatus.py to generate code in separate,
specified files, as well as generates the entire list of errors of a
specified file rather than checking which errors already exist. The
third patch is the file which should be used.

The fourth patch adds this all to the build system, so the files are
automatically generated on build.

Also attached is a little script which I used to convince myself that I
haven't missed out on any errors and that they're all defined to the
correct numbers. What is mostly important is that this checks that all
definitions in the old ntstatus.h are present with the correct numbers
in the new ntstatus_gen.h. This comes up with a bunch that aren't. These
ones are either defined incorrectly (e.g. STATUS_PENDING should be
NT_STATUS_PENDING), in which case I redefined them to point to the
correct version, or they're not present in the table used to generate
it. Ones that aren't present in the table and are used somewhere in
Samba, I defined in ntstatus.h. Ones that aren't used at all, I removed.
The ones that are removed are given in the final commit message. I went
through this list one-by-one, and it would be a good idea for any
reviewers to check it as well. The second part of the script also shows
us which statuses I removed.

I also had to manually add some entries in nterr.c. These entries were
put in a new array, "special_errs", which is included in all of the
relevant functions.

The final three patches fix up a use of statuses and use these generated
statuses in samba-tool/domain.

Guenther: If my first patch is alright, would you be opposed to merging
my first patch into your patch?

Please review.

Thanks,
Bob


On 31/10/16 15:32, Andrew Bartlett wrote:
> On Mon, 2016-10-10 at 12:25 +0300, Alexander Bokovoy wrote:
>> On ma, 10 loka 2016, Stefan Metzmacher wrote:
>>> Am 10.10.2016 um 04:12 schrieb Andrew Bartlett:
>>>> On Thu, 2016-10-06 at 15:19 -0700, Jeremy Allison wrote:
>>>>> On Fri, Sep 30, 2016 at 06:25:15AM +0200, Günther Deschner
>>>>> wrote:
>>>>>> Hi,
>>>>>>
>>>>>> please review and push.
>>>>>>
>>>>>> Thanks,
>>>>>> Guenther
>>>>> Hi Guenther, I'm still working on my python - but get:
>>>>>
>>>>> ../libcli/util/py_ntstatus.c:17:6: error: no previous prototype
>>>>> for
>>>>> ‘initntstatus’ [-Werror=missing-prototypes]
>>>>>  void initntstatus(void)
>>>>>
>>>>> when I try and build with this. Not sure where the
>>>>> missing proto should go...
>>>> Nowhere (or just above it, to shut up the warning).
>>>>
>>>> Because this is essentially a plugin (to python), it is like our
>>>> own
>>>> modules where the caller (python) works out the name and finds it
>>>> with
>>>> dlopen/dlsym stuff, rather than via a header. 
>>>>
>>>> The only thing I would ask for is some spaces in this:
>>>> +    print "writing new headerfile: %s"%pythonfile_name
>>>> +    out_file = open(pythonfile_name,"w")
>>>>
>>>> eg
>>>>
>>>> +    print "writing new headerfile: %s" % pythonfile_name
>>>> +    out_file = open(pythonfile_name, "w")
>>>>
>>>> Sorry for the nitpick, but otherwise, it looks good to me, and we
>>>> can
>>>> work from here to make some python exceptions so we can raise
>>>> something
>>>> sensible when we need to return an NTSTATUS error from C.
>>> Because of things like that and we may also want to have some
>>> functions
>>> translate from between NTSTATUS, WERROR and HRESULT, we may want to
>>> have a generic, samba.errors module that contains all of them.
>>>
>>> So we may want to have a py_ntstatus_init(PyObject *m),
>>> py_werror_init(PyObject *m) and py_hresult_init(PyObject *m)
>>> which are called from a handwritten initerrors() function.
>>>
>>> And I'd like to use a define instead of integer.
>>>
>>> PyModule_AddObject(m, "NT_STATUS_SUCCESS",
>>> ndr_PyLong_FromUnsignedLongLong(0x00000000));
>>>
>>> should be
>>>
>>> PyModule_AddObject(m, "NT_STATUS_SUCCESS",
>>> 		   ndr_PyLong_FromUnsignedLongLong(NT_STATUS_V(NT_STATU
>>> S_SUCCESS)));
>>>
>>> Or we use a define to make the lines shorter.
>>>
>>> #define _ADD_CODE(m, v) do {\
>>> 	PyModule_AddObject(m, #v, \
>>> 		ndr_PyLong_FromUnsignedLongLong(NT_STATUS_V(v))); \
>>> } while(0)
>> Agreed. This is one area which made my life on FreeIPA side miserable
>> when tracking error codes. Having them unified is welcomed.
> Ping!
>
> It would still be really good to get this in to master!
>
> Andrew Bartlett
>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: ntstatus-compare-script.py
Type: text/x-python
Size: 4381 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20170113/5cc0922f/ntstatus-compare-script-0001.py>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-errors-generate-python-error-codes-for-NTSTATUS.patch
Type: text/x-patch
Size: 2793 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20170113/5cc0922f/0001-errors-generate-python-error-codes-for-NTSTATUS-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-errors-fix-generate-python-error-codes-for-NTSTATUS.patch
Type: text/x-patch
Size: 2621 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20170113/5cc0922f/0002-errors-fix-generate-python-error-codes-for-NTSTATUS-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-errors-gen_ntstatus-generate-error-codes-in-specifie.patch
Type: text/x-patch
Size: 11218 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20170113/5cc0922f/0003-errors-gen_ntstatus-generate-error-codes-in-specifie-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-errors-gen_ntstatus-add-error-table-for-generation-s.patch
Type: text/x-patch
Size: 242594 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20170113/5cc0922f/0004-errors-gen_ntstatus-add-error-table-for-generation-s-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0005-errors-add-gen_ntstatus.py-to-build-system.patch
Type: text/x-patch
Size: 499278 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20170113/5cc0922f/0005-errors-add-gen_ntstatus.py-to-build-system-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0006-errors-remove-duplicate-error-string-table-entries.patch
Type: text/x-patch
Size: 1644 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20170113/5cc0922f/0006-errors-remove-duplicate-error-string-table-entries-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0007-python-Add-python-module-with-NTSTATUS-constants.patch
Type: text/x-patch
Size: 940 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20170113/5cc0922f/0007-python-Add-python-module-with-NTSTATUS-constants-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0008-samba-tool-domain-change-incorrect-NT_STATUS-to-WERR.patch
Type: text/x-patch
Size: 2341 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20170113/5cc0922f/0008-samba-tool-domain-change-incorrect-NT_STATUS-to-WERR-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0009-samba-tool-domain-use-generated-ntstatus-rather-than.patch
Type: text/x-patch
Size: 8642 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20170113/5cc0922f/0009-samba-tool-domain-use-generated-ntstatus-rather-than-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0010-samba-tool-domain-catch-NTSTATUSError-rather-than-Ru.patch
Type: text/x-patch
Size: 7071 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20170113/5cc0922f/0010-samba-tool-domain-catch-NTSTATUSError-rather-than-Ru-0001.bin>


More information about the samba-technical mailing list