[PATCH] Improved adding images

[Jan Darowski]

I've cleaned whitespaces, removed completely qstrdump (left one
copy_string where it was needed), merged 2 later patches into one (3.
was just changing message from 2.), changed author of the commits to
match Signed-off-by.

2015-03-15 15:31 GMT+01:00 Dirk Hohndel <dirk at hohndel.org>:
> On Sun, Mar 15, 2015 at 11:01:06AM +0100, Jan Darowski wrote:
>> 1) To be honest I'm not sure where it comes from, seems like
>> QtDesigner chenged it...
>
> OK, that makes sense.
>
>> 2) I think this qstrdup can and should be removed...  I used qstrdup
>> instead of strdup (they work the same) because somewhere else qstrdup
>> was used.
>
> In general we should use copy_string().
> A quick grep shows that we have one single other use of qstrdup and
> usually use our own copy_string (and occasionally use strdup). The
> difference is that copy_string handles NULL pointers while strdup doesn't.
> In this case you shouldn't have the risk of getting a NULL pointer back
> but using copy_string might still be the most consistent choice.
>
> From looking at the code in this particular instance the copy isn't needed
> because the fileNames QStringList is in scope until the end of this
> function. Since the called function only references the string but doesn't
> keep the pointer that is passed in around there is no issue.
>
>> It's good that you pointed it out because most of these
>> qstrdup uses cause mem leaks right now.
>
> Well, usually they are there because either the original string would go
> out of scope or because we want to make sure that we can free the pointer
> later. Subsurface does leak some memory here and there, but I don't think
> "most of" the string copying is incorrect. There might be a few calls to
> free missing here and there (even though we do our occasional attempts to
> find and add those).
>
>> In the evening I will send corrected version of these patches, sorry
>> for the bugs.
>
> Thanks. Just in case this wasn't obvious - your patches looked good. What
> I did was mostly nitpick since this was your first submission to
> Subsurface...
>
> One more small comment - your From line (which turns into the "Author")
> and your Signed-of-by line don't agree with each other.
> Eltharan <johny.dar at gmail.com> / Jan Darowski <jan.darowski at gmail.com>
>
> It would look better if you could be consistent. We have many contributors
> who use several email addresses and I usually don't mind, but again, as
> you set things up, this might be an easy thing to fix.
>
> Thanks
>
> /D
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Refactored-image-timestamp-checking.patch
Type: text/x-patch
Size: 6299 bytes
Desc: not available
URL: <http://lists.subsurface-divelog.org/pipermail/subsurface/attachments/20150315/6899a1c4/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Added-warning-when-not-all-images-can-be-added.patch
Type: text/x-patch
Size: 5561 bytes
Desc: not available
URL: <http://lists.subsurface-divelog.org/pipermail/subsurface/attachments/20150315/6899a1c4/attachment-0001.bin>