Re: [PATCH 5/7] simple-ipc/ipc-win32: add Windows ACL to named pipe

On Wed, Sep 15 2021, Jeff Hostetler via GitGitGadget wrote:

+struct my_sa_data
+{
+	PSID pEveryoneSID;
+	PACL pACL;
+	PSECURITY_DESCRIPTOR pSD;
+	LPSECURITY_ATTRIBUTES lpSA;
+};
+
+static void init_sa(struct my_sa_data *d)
+{
+	memset(d, 0, sizeof(*d));
+}
+
+static void release_sa(struct my_sa_data *d)
+{
+	if (d->pEveryoneSID)
+		FreeSid(d->pEveryoneSID);
+	if (d->pACL)
+		LocalFree(d->pACL);
+	if (d->pSD)
+		LocalFree(d->pSD);
+	if (d->lpSA)
+		LocalFree(d->lpSA);
+
+	memset(d, 0, sizeof(*d));
+}

[...]

+fail:
+	release_sa(d);
+	return NULL;
+}
+
 static HANDLE create_new_pipe(wchar_t *wpath, int is_first)
 {
 	HANDLE hPipe;
 	DWORD dwOpenMode, dwPipeMode;
-	LPSECURITY_ATTRIBUTES lpsa = NULL;
+	struct my_sa_data my_sa_data;
+
+	init_sa(&my_sa_data);
 

[...]

 	hPipe = CreateNamedPipeW(wpath, dwOpenMode, dwPipeMode,
-				 PIPE_UNLIMITED_INSTANCES, 1024, 1024, 0, lpsa);
+				 PIPE_UNLIMITED_INSTANCES, 1024, 1024, 0,
+				 my_sa_data.lpSA);
+
+	release_sa(&my_sa_data);
 
 	return hPipe;
 }

Just a nit on the init pattern, since we always allocate this on the
stack (this as all the relevant "struct my_sa_data") I'd have thought to
see:

    #define INIT_MY_SA_DATA { 0 }
    [...]
    struct my_sa_data my_sa_data = INIT_MY_SA_DATA;

Which gets rid of the need for an init_sa() function.

Also having the release_sa() do a memset() is a bit odd, usually we have
a reset*() function do that if the intent is to re-use, but it doesn't
appear to be in this case, and we don't return this data anywhere, do
we?