[PATCH] more FSRVP & additionally HRESULT error

David Disseldorp ddiss at suse.de
Tue Mar 25 06:38:52 MDT 2014


Hi Noel,

On Tue, 25 Mar 2014 10:35:39 +0000, Noel Power wrote:

...
> > It would be also good if you could split
> > http://cgit.freedesktop.org/~noelp/noelp-samba/commit/?h=heave_in_hresult-v2&id=be6cc634b31777044e4c7d5c93d0124bc41ad1b7
> > to that the libcli/util/ changes are in there own commit.
> > The rest should be combined with
> > http://cgit.freedesktop.org/~noelp/noelp-samba/commit/?h=heave_in_hresult-v2&id=d0e9479324893f35b52012d7bddb5f244471a94a
> done, please see
> http://cgit.freedesktop.org/~noelp/noelp-samba/log/?h=heave_in_hresult-v4

+typedef struct {uint32_t w;} HRESULT;
+#define HRES_ERROR(x) ((HRESULT) { x })
+#define HRES_ERROR_V(x) ((x).w)

Please use a struct member var other than "w" or "v", so that these
macros can't be mixed with the ntstatus and werror helpers.

+#define HRES_IS_EQUAL(x,y) (HRES_ERROR_V(x) == HRES_ERROR_V(y))

It's probably worth adding an HRES_IS_OK helper macro here too.

> 
> On 19/03/14 13:30, Stefan (metze) Metzmacher wrote:
> [...]
> >> One thing I noticed is that you imported
> >> NT_STATUS_RPC_NT_* values, while we already have some of them
> >> as NT_STATUS_RPC_*. Can you change the import to use just NT_STATUS_RPC_*
> >> and remove the once we already have?
> > Please also change NT_STATUS_EPT_NT_* into just NT_STATUS_EPT_*
> >
> also done please see
> http://cgit.freedesktop.org/~noelp/noelp-samba/log/?h=heave_in_ntstatus-v2
> 
> note: I did unexpectedly find a duplicate with the NT_STATUS errors
> after transforming the names as you requested. However, it appears that
> the duplicate resulted from the original constant having an incorrect
> error code defined ( see 3e4aead159ca0b6445a9e99b5b54b1cb58a55559 in
> branch above )

Good catch. These heave_in_ntstatus-v2 changes look good to me.
Reviewed-by: David Disseldorp <ddiss at samba.org>

Cheers, David


More information about the samba-technical mailing list