Created attachment 296245 [details] [review]
smb: Fix a memory leak

Created attachment 296246 [details] [review]
channel: Fix a couple of leaks

Fix leaks when sending a GFileInfo through a channel or sending an
error.

This can be seen by running gvfsd through valgrind and doing
query_info_on_{read,write} or truncating with a negative size.

Created attachment 296311 [details] [review]
dav: Fix a few memory leaks

Thanks for the fixes. Extension for reviews is broken currently, so:

(In reply to Ross Lagerwall from comment #1)
> Created attachment 296245 [details] [review] [review]
> smb: Fix a memory leak

Looks good, however wouldn't be better to have only one g_free after the if-else statement?

(In reply to Ross Lagerwall from comment #3)
> Created attachment 296311 [details] [review] [review]
> dav: Fix a few memory leaks

Looks good!

Review of attachment 296246 [details] [review]:

Valgrind reports lot of things, but I don't see this leaks. However the patch seems good to me :-)

(In reply to Ondrej Holy from comment #5)
> Review of attachment 296246 [details] [review] [review]:
> 
> Valgrind reports lot of things, but I don't see this leaks. However the
> patch seems good to me :-)

Well you have to make sure you run valgrind with the right options.  This is how I do it:
$ pkill gvfs; GVFS_DEBUG=all valgrind --trace-children=yes --leak-check=full --show-reachable=yes '--log-file=val-%p' daemon/gvfsd

$ gvfs-mount smb://...
$ ./truncate smb://.../some/file -1
$ ./query-read smb://.../some/file
$ gvfs-mount -u smb://...

Valgrind will then output a whole bunch of stuff to a val-pid file. If you search for "definitely", you will see:

This is lost for each query_info_{read,write}:
==4682== 512 bytes in 1 blocks are definitely lost in loss record 2,568 of 2,599
==4682==    at 0x4C2C29E: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==4682==    by 0x67D0BE8: g_realloc (gmem.c:162)
==4682==    by 0x579727C: array_resize (gmemoryoutputstream.c:570)
==4682==    by 0x579747B: g_memory_output_stream_write (gmemoryoutputstream.c:649)
==4682==    by 0x579DC90: g_output_stream_write (goutputstream.c:219)
==4682==    by 0x5787B12: g_filter_output_stream_write (gfilteroutputstream.c:265)
==4682==    by 0x579DC90: g_output_stream_write (goutputstream.c:219)
==4682==    by 0x579DDC8: g_output_stream_write_all (goutputstream.c:278)
==4682==    by 0x57626F0: g_data_output_stream_put_uint16 (gdataoutputstream.c:332)
==4682==    by 0x52A60F3: put_string (gvfsfileinfo.c:43)
==4682==    by 0x52A63DE: gvfs_file_info_marshal (gvfsfileinfo.c:126)
==4682==    by 0x5071F49: g_vfs_channel_send_info (gvfschannel.c:696)

This is lost for each channel error:
==4966== 50 bytes in 1 blocks are definitely lost in loss record 1,369 of 2,397
==4966==    at 0x4C29F90: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==4966==    by 0x67D0B08: g_malloc (gmem.c:97)
==4966==    by 0x5073DF0: g_error_to_daemon_reply (gvfsdaemonutils.c:57)
==4966==    by 0x5071F06: g_vfs_channel_send_error (gvfschannel.c:682)
==4966==    by 0x507B73F: send_reply (gvfsjobtruncate.c:97)
==4966==    by 0x632AC96: g_cclosure_marshal_VOID__VOIDv (gmarshal.c:115)
==4966==    by 0x6328334: g_type_class_meta_marshalv (gclosure.c:988)
==4966==    by 0x6327EF7: _g_closure_invoke_va (gclosure.c:831)
==4966==    by 0x6342F71: g_signal_emit_valist (gsignal.c:3201)
==4966==    by 0x63440DB: g_signal_emit (gsignal.c:3348)
==4966==    by 0x5074A0D: g_vfs_job_send_reply (gvfsjob.c:236)
==4966==    by 0x5074B99: g_vfs_job_failed_from_error (gvfsjob.c:282)

Pushed to master as 07fc7a65258bce9c3ad950f9adc0e6c66e2b0835 with the suggested changes. Thanks for the review.

(In reply to Ross Lagerwall from comment #6)
> (In reply to Ondrej Holy from comment #5)
> > Review of attachment 296246 [details] [review] [review] [review]:
> > 
> > Valgrind reports lot of things, but I don't see this leaks. However the
> > patch seems good to me :-)
> 
> Well you have to make sure you run valgrind with the right options. 

That was the problem probably, thanks for the example :-)