[PATCH] Improved adding images

[Dirk Hohndel]

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