[Fwd: Re: [PATCH] Minor cleanup of libnet_LookupName_recv] || 2nd reviewer, please
Swen Schillig
swen at vnet.ibm.com
Mon Feb 26 15:52:16 UTC 2018
Hi Ralph
On Mon, 2018-02-26 at 16:47 +0100, Ralph Böhme wrote:
> Hi Swen,
>
> On Mon, Feb 26, 2018 at 04:17:38PM +0100, Swen Schillig via samba-
> technical wrote:
> > Anyway, as I said above, I have no strong feelings about using or
> > not
> > using "unlikely" here. I f the two of you prefer, I will remove it.
>
> yes, please submit an updated patchset without the unlikely and I
> will be happy
> to re-review. :)
Sure, here we go.
Thanks in advance for your support.
Chers Swen
-------------- next part --------------
From 8ce646df7827b7ab3c6de07c1ed5c20c4c24ed50 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..a8d41f45335 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);
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 5cea77a9d8da7d519be92e2a5f3aa71e67b4ec21 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 | 69 ++++++++++++++++++++++++------------------
1 file changed, 39 insertions(+), 30 deletions(-)
diff --git a/source4/libnet/libnet_lookup.c b/source4/libnet/libnet_lookup.c
index a8d41f45335..c4befcacda9 100644
--- a/source4/libnet/libnet_lookup.c
+++ b/source4/libnet/libnet_lookup.c
@@ -381,42 +381,51 @@ NTSTATUS libnet_LookupName_recv(struct composite_context *c, TALLOC_CTX *mem_ctx
struct libnet_LookupName *io)
{
NTSTATUS status;
- struct lookup_name_state *s;
+ struct lookup_name_state *s = NULL;
+ struct lsa_RefDomainList *domains = NULL;
+ struct lsa_TransSidArray *sids = NULL;
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;
+ }
+
+ domains = *s->lookup.out.domains;
+ 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) {
+ 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 05e589a5b216973e093b66795815160039c8a65f 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 leaking 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 c4befcacda9..63ce6015b66 100644
--- a/source4/libnet/libnet_lookup.c
+++ b/source4/libnet/libnet_lookup.c
@@ -419,9 +419,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 (io->out.sid == NULL) {
+ status = 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 (io->out.sidstr == NULL) {
+ status = NT_STATUS_NO_MEMORY;
+ goto done;
+ }
}
success:
--
2.14.3
More information about the samba-technical
mailing list