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

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

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?