[PATCH] Minor cleanup of libnet_LookupName_recv
Christof Schmitt
cs at samba.org
Thu Feb 1 20:40:01 UTC 2018
On Thu, Feb 01, 2018 at 09:46:33AM +0100, Swen Schillig wrote:
> As suggested the patch is split into 3 individual patches.
>
> Please review.
>
> Thanks in advance for your support !.
Can you check the compile errors?
./configure --picky-developer --enable-selftest && make -j
results in:
In file included from ../source4/include/includes.h:23:0,
from ../source4/libnet/libnet_lookup.c:24:
../source4/libnet/libnet_lookup.c: In function ‘libnet_LookupName_recv’:
../lib/replace/../replace/replace.h:802:39: error: lvalue required as unary ‘&’ operand
#define ZERO_STRUCT(x) memset((char *)&(x), 0, sizeof(x))
^
../source4/libnet/libnet_lookup.c:387:2: note: in expansion of macro ‘ZERO_STRUCT’
ZERO_STRUCT(&io->out);
^
../source4/libnet/libnet_lookup.c:401:2: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement]
struct lsa_RefDomainList *domains = *s->lookup.out.domains;
^
../source4/libnet/libnet_lookup.c:421:4: error: ‘statust’ undeclared (first use in this function)
statust = NT_STATUS_NO_MEMORY;
^
../source4/libnet/libnet_lookup.c:421:4: note: each undeclared identifier is reported only once for each function it appears in
cc1: all warnings being treated as errors
Waf: Leaving directory `/home/cschmit/code/samba/bin'
Build failed: -> task failed (err #1):
{task: cc libnet_lookup.c -> libnet_lookup_1.o}
make: *** [all] Error 1
> From 25467dee127589e58dd1e235e23e00e6767323ec Mon Sep 17 00:00:00 2001
> From: Swen Schillig <swen at vnet.ibm.com>
> Date: Fri, 26 Jan 2018 13:28:58 +0100
> Subject: [PATCH 1/3] Zero libnet_LookupName out struct before using
>
> Zero libnet_LookupName out struct before setting results,
> preventing false result interpretation.
>
> Signed-off-by: Swen Schillig <swen at vnet.ibm.com>
> ---
> source4/libnet/libnet_lookup.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/source4/libnet/libnet_lookup.c b/source4/libnet/libnet_lookup.c
> index bdb99950493..1e17d794782 100644
> --- a/source4/libnet/libnet_lookup.c
> +++ b/source4/libnet/libnet_lookup.c
> @@ -384,14 +384,11 @@ NTSTATUS libnet_LookupName_recv(struct composite_context *c, TALLOC_CTX *mem_ctx
> struct lookup_name_state *s;
>
> status = composite_wait(c);
> + ZERO_STRUCT(&io->out);
The argument should be io->out.
>
> if (NT_STATUS_IS_OK(status)) {
> s = talloc_get_type(c->private_data, struct lookup_name_state);
>
> - io->out.rid = 0;
> - io->out.sid = NULL;
> - io->out.sidstr = NULL;
> -
> if (*s->lookup.out.count > 0) {
> struct lsa_RefDomainList *domains = *s->lookup.out.domains;
> struct lsa_TransSidArray *sids = s->lookup.out.sids;
> --
> 2.14.3
>
>
> From 58d44833dd1594dc0031853c695615030f19274e Mon Sep 17 00:00:00 2001
> From: Swen Schillig <swen at vnet.ibm.com>
> Date: Thu, 1 Feb 2018 09:02:25 +0100
> Subject: [PATCH 2/3] Minor cleanup of libnet_LookupName_recv
>
> Reduce indentation level and comply with 80 column rule.
>
> Signed-off-by: Swen Schillig <swen at vnet.ibm.com>
> ---
> source4/libnet/libnet_lookup.c | 65 +++++++++++++++++++++++-------------------
> 1 file changed, 36 insertions(+), 29 deletions(-)
>
> diff --git a/source4/libnet/libnet_lookup.c b/source4/libnet/libnet_lookup.c
> index 1e17d794782..14f6a3a6306 100644
> --- a/source4/libnet/libnet_lookup.c
> +++ b/source4/libnet/libnet_lookup.c
> @@ -386,37 +386,44 @@ NTSTATUS libnet_LookupName_recv(struct composite_context *c, TALLOC_CTX *mem_ctx
> status = composite_wait(c);
> ZERO_STRUCT(&io->out);
>
> - if (NT_STATUS_IS_OK(status)) {
> - s = talloc_get_type(c->private_data, struct lookup_name_state);
> -
> - if (*s->lookup.out.count > 0) {
> - struct lsa_RefDomainList *domains = *s->lookup.out.domains;
> - struct lsa_TransSidArray *sids = s->lookup.out.sids;
> -
> - if (domains == NULL || sids == NULL) {
> - status = NT_STATUS_UNSUCCESSFUL;
> - io->out.error_string = talloc_asprintf(mem_ctx, "Error: %s", nt_errstr(status));
> - goto done;
> - }
> -
> - if (sids->count > 0) {
> - io->out.rid = sids->sids[0].rid;
> - io->out.sid_type = sids->sids[0].sid_type;
> - if (domains->count > 0) {
> - io->out.sid = dom_sid_add_rid(mem_ctx, domains->domains[0].sid, io->out.rid);
> - NT_STATUS_HAVE_NO_MEMORY(io->out.sid);
> - io->out.sidstr = dom_sid_string(mem_ctx, io->out.sid);
> - NT_STATUS_HAVE_NO_MEMORY(io->out.sidstr);
> - }
> - }
> - }
> -
> - io->out.error_string = talloc_strdup(mem_ctx, "Success");
> -
> - } else if (!NT_STATUS_IS_OK(status)) {
> - io->out.error_string = talloc_asprintf(mem_ctx, "Error: %s", nt_errstr(status));
> + if (!NT_STATUS_IS_OK(status)) {
> + io->out.error_string = talloc_asprintf(mem_ctx, "Error: %s",
> + nt_errstr(status));
> + goto done;
> + }
> +
> + s = talloc_get_type(c->private_data, struct lookup_name_state);
> +
> + if (*s->lookup.out.count == 0) {
> + goto success;
> + }
> +
> + struct lsa_RefDomainList *domains = *s->lookup.out.domains;
> + struct lsa_TransSidArray *sids = s->lookup.out.sids;
The variable definition has to go to the top of the function. Also see
README.Coding about pointer initialization.
> +
> + if (domains == NULL || sids == NULL) {
> + status = NT_STATUS_UNSUCCESSFUL;
> + io->out.error_string = talloc_asprintf(mem_ctx, "Error: %s",
> + nt_errstr(status));
> + goto done;
> + }
> +
> + if (sids->count == 0) {
> + goto success;
> + }
> +
> + io->out.rid = sids->sids[0].rid;
> + io->out.sid_type = sids->sids[0].sid_type;
> + if (domains->count > 0) {
> + io->out.sid = dom_sid_add_rid(mem_ctx, domains->domains[0].sid,
> + io->out.rid);
> + NT_STATUS_HAVE_NO_MEMORY(io->out.sid);
> + io->out.sidstr = dom_sid_string(mem_ctx, io->out.sid);
> + NT_STATUS_HAVE_NO_MEMORY(io->out.sidstr);
> }
>
> +success:
> + io->out.error_string = talloc_strdup(mem_ctx, "Success");
> done:
> talloc_free(c);
> return status;
> --
> 2.14.3
>
>
> From d41273d7dc45eb86583fe049cca1efe5e5566945 Mon Sep 17 00:00:00 2001
> From: Swen Schillig <swen at vnet.ibm.com>
> Date: Thu, 1 Feb 2018 09:39:02 +0100
> Subject: [PATCH 3/3] Replace NT_STATUS_HAVE_NO_MEMORY macro
>
> Replaced NT_STATUS_HAVE_NO_MEMORY macro and fixed
> memory leakin error-path.
>
> Signed-off-by: Swen Schillig <swen at vnet.ibm.com>
> ---
> source4/libnet/libnet_lookup.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/source4/libnet/libnet_lookup.c b/source4/libnet/libnet_lookup.c
> index 14f6a3a6306..bf58bac8748 100644
> --- a/source4/libnet/libnet_lookup.c
> +++ b/source4/libnet/libnet_lookup.c
> @@ -417,9 +417,15 @@ NTSTATUS libnet_LookupName_recv(struct composite_context *c, TALLOC_CTX *mem_ctx
> if (domains->count > 0) {
> io->out.sid = dom_sid_add_rid(mem_ctx, domains->domains[0].sid,
> io->out.rid);
> - NT_STATUS_HAVE_NO_MEMORY(io->out.sid);
> + if (unlikely(io->out.sid == NULL)) {
> + statust = NT_STATUS_NO_MEMORY;
> + goto done;
> + }
> io->out.sidstr = dom_sid_string(mem_ctx, io->out.sid);
> - NT_STATUS_HAVE_NO_MEMORY(io->out.sidstr);
> + if (unlikely(io->out.sidstr == NULL)) {
> + statust = NT_STATUS_NO_MEMORY;
The variable name is status, not statust.
> + goto done;
> + }
> }
>
> success:
Christof
More information about the samba-technical
mailing list