[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).
------------------------------------------------------------------
1).
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;
+ }
------------------------------------------------------------------
2).
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
dependency.
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 !
*GREAT WORK* !!
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 !
Jeremy.
More information about the samba-technical
mailing list