[PATCH v3] File Server Remote VSS Protocol server

Jeremy Allison jra at samba.org
Thu Mar 26 17:13:34 MDT 2015

On Thu, Mar 26, 2015 at 03:31:26PM +0100, David Disseldorp wrote:
> This patch-set adds a new File Server Remote VSS (Volume Shadow Copy
> Service) Protocol server, test infrastructure, documentation, and
> corresponding VFS layer changes to handle remote share snapshot
> requests.
> Feedback appreciated.

A couple of nitpicky things (sorry).

In static NTSTATUS snapper_create_snap_pack()

+       enc_ctx = talloc_new(mem_ctx);
+       if (enc_ctx == NULL) {
+               return NT_STATUS_NO_MEMORY;
+       }
+       msg = dbus_message_new_method_call("org.opensuse.Snapper",
+                                          "/org/opensuse/Snapper",
+                                          "org.opensuse.Snapper",
+                                          "CreateSingleSnapshot");
+       if (msg == NULL) {
+               DEBUG(0, ("failed to create req msg\n"));

*** MISSING****
		 talloc_free(enc_ctx); here.

+               return NT_STATUS_NO_MEMORY;
+       }

In +uint32_t _fss_SetContext(struct pipes_struct *p,
+                        struct fss_SetContext *r)

+       talloc_free(fss_global.seq_tmr);        /* kill timer if running */
+       fss_seq_tout_set(fss_global.mem_ctx, 180, NULL, &fss_global.seq_tmr);

Use TALLOC_FREE(fss_global.seq_tmr) to ensure it's
correctly NULL'ed out. If fss_seq_tout_set() fails
then fss_global.seq_tmr is left with a bad pointer
if you just call talloc_free().

I looked into the code inside fss_seq_tout_set() and
currently the talloc_free() usage is safe - it can't
fail without setting fss_global.seq_tmr to NULL at
present, but code changes over time, and whoever
changes fss_seq_tout_set() may not notice the

You already do this (TALLOC_FREE(fss_global.seq_tmr);)
in all other cases I can see.

Right now I can't see any other issues at all !


I'm going to run a 'make test' and if all is well
(I'll reply by tomorrow) please fix these and then
push with my 'Reviewed-by:'.

Cheers and thanks a *LOT* for your patience on this !


More information about the samba-technical mailing list