Peter Eisentraut writes:
> With the attached patch, that warning goes way, and the logic is
> arguably slightly clearer, too.
No objection.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
htt
On Wed, 2013-11-13 at 12:43 -0500, Tom Lane wrote:
> Kevin Grittner writes:
> > If nobody objects, I'll fix that small memory leak in the
> > regression test driver. Hopefully someone more familiar with
> > pg_basebackup will fix the double-free (and related problems
> > mentioned by Tom) in strea
Kevin Grittner writes:
> If nobody objects, I'll fix that small memory leak in the
> regression test driver. Hopefully someone more familiar with
> pg_basebackup will fix the double-free (and related problems
> mentioned by Tom) in streamutil.c.
Here's a less convoluted (IMHO) approach to the pa
Tom Lane wrote:
> No, this isn't about test code vs production, it's about not bothering
> to free memory explicitly when a program is about to terminate. Alvaro
> is suggesting that the proposed addition to pg_regress.c is just a waste
> of cycles. IMO it's not that big a deal either way in th
Jeffrey Walton writes:
> I did not check any with the long path lengths, but the
> `pqsecure_write` in fe-secure.c looks valid to me. `spinfo` is
> declared, Clang builds/finds the path, then the unitializaed `spinfo`
> is used in `RESTORE_SIGPIPE(conn, spinfo);`.
It's junk AFAICS, though I will
On Tue, 2013-11-12 at 19:11 -0500, Jeffrey Walton wrote:
> The reports being generated with Clang 3.3 on Postgres 9.3.1 are
> different that posted. For example, french.c is not listed in the
> Clang 3.3 reports. What version of Clang is used in the online report?
LLVM 3.0
--
Sent via pgsql-ha
Jeffrey Walton writes:
> On Tue, Nov 12, 2013 at 7:11 PM, Alvaro Herrera
> wrote:
>> We have marked a large number of memory leak reports by Coverity in
>> initdb and other short-lived programs as false positive, on the grounds
>> that there's no point in freeing memory in a program that's about
On Tue, Nov 12, 2013 at 7:11 PM, Alvaro Herrera
wrote:
> Kevin Grittner escribió:
>
>> These both seemed legitimate to me. Patch attached. Any
>> objections to applying it? I realize the memory leak is a tiny one
>> in the regression testing code, so it could never amount to enough
>> to matter
On Tue, Nov 12, 2013 at 5:19 PM, Peter Eisentraut wrote:
> On 11/12/13, 8:18 AM, Kevin Grittner wrote:
>> Here is the summary of what was reported:
>>
>> All Bugs: 313
>
>> Does anything stand out as something that is particularly worth
>> looking into? Does anything here seem worth assuming is
Kevin Grittner escribió:
> These both seemed legitimate to me. Patch attached. Any
> objections to applying it? I realize the memory leak is a tiny one
> in the regression testing code, so it could never amount to enough
> to matter; but it seems worth fixing just to avoid noise in code
> analy
On Tue, Nov 12, 2013 at 6:04 PM, Kevin Grittner wrote:
> Peter Eisentraut wrote:
>
>> I have tracked scan-build for some time, and I'm sure that almost
>> all of these bugs are false positives at this point.
>
> From poking around, I agree. One particular error I noticed that
> it makes a lot is
Peter Eisentraut wrote:
> I have tracked scan-build for some time, and I'm sure that almost
> all of these bugs are false positives at this point.
From poking around, I agree. One particular error I noticed that
it makes a lot is that in a loop it says that an assigned value is
not referenced i
On 11/12/13, 8:18 AM, Kevin Grittner wrote:
> Here is the summary of what was reported:
>
> All Bugs: 313
> Does anything stand out as something that is particularly worth
> looking into? Does anything here seem worth assuming is completely
> bogus because of the Coverity and Valgrind passes?
On Tue, Nov 12, 2013 at 3:35 PM, Andres Freund wrote:
> On 2013-11-12 15:33:13 -0500, Jeffrey Walton wrote:
>> On Tue, Nov 12, 2013 at 3:25 PM, Andres Freund
>> wrote:
>> > On 2013-11-12 15:17:18 -0500, Jeffrey Walton wrote:
>> > ...
>> > It might not recognize our Assert() because it expands as
On Tue, Nov 12, 2013 at 9:38 AM, Tom Lane wrote:
> Kevin Grittner writes:
>> Does anything stand out as something that is particularly worth
>> looking into? Does anything here seem worth assuming is completely
>> bogus because of the Coverity and Valgrind passes?
>
> I thought most of it was ob
On 2013-11-12 15:33:13 -0500, Jeffrey Walton wrote:
> On Tue, Nov 12, 2013 at 3:25 PM, Andres Freund wrote:
> > On 2013-11-12 15:17:18 -0500, Jeffrey Walton wrote:
> >> On Tue, Nov 12, 2013 at 9:38 AM, Tom Lane wrote:
> >> > ...
> >> > One thought for the Clang people is that most of the reports
On Tue, Nov 12, 2013 at 3:25 PM, Andres Freund wrote:
> On 2013-11-12 15:17:18 -0500, Jeffrey Walton wrote:
>> On Tue, Nov 12, 2013 at 9:38 AM, Tom Lane wrote:
>> > ...
>> > One thought for the Clang people is that most of the reports such as "null
>> > pointer dereference" presumably mean "I thi
On 2013-11-12 15:17:18 -0500, Jeffrey Walton wrote:
> On Tue, Nov 12, 2013 at 9:38 AM, Tom Lane wrote:
> > ...
> >
> > One thought for the Clang people is that most of the reports such as "null
> > pointer dereference" presumably mean "I think I see an execution path
> > whereby we could get here
On Tue, Nov 12, 2013 at 9:38 AM, Tom Lane wrote:
> ...
>
> One thought for the Clang people is that most of the reports such as "null
> pointer dereference" presumably mean "I think I see an execution path
> whereby we could get here with a null pointer". If so, it'd be awfully
> helpful if the c
Kevin Grittner writes:
> Kevin Grittner wrote:
>> Memory Error
>> Double free: 1
>> Memory leak: 1
> These both seemed legitimate to me. Patch attached. Any
> objections to applying it? I realize the memory leak is a tiny one
> in the regression testing code, so it could never amount
Kevin Grittner wrote:
> Memory Error
> Double free: 1
> Memory leak: 1
These both seemed legitimate to me. Patch attached. Any
objections to applying it? I realize the memory leak is a tiny one
in the regression testing code, so it could never amount to enough
to matter; but it seems wo
* Tom Lane (t...@sss.pgh.pa.us) wrote:
> I think Coverity does that, or at least I've seen output from some tool
> that does it.
Coverity does provide the path (including going through multiple
interations of a loop, if applicable). Doesn't make it perfect, sadly,
but I've been trying to feed bac
Kevin Grittner writes:
> Does anything stand out as something that is particularly worth
> looking into? Does anything here seem worth assuming is completely
> bogus because of the Coverity and Valgrind passes?
I thought most of it was obvious junk: if there were actually
uninitialized-variable
* Peter Geoghegan (p...@heroku.com) wrote:
> I seem to also recall
> Coverity correctly handling that, although perhaps I'm unfairly
> crediting them with taking advantage of the abort() trick because of
> the state of Postgres when I tried each of those two tools - it might
> be that scan-build *w
Kevin Grittner wrote:
> Logic error
> Stack address stored into global variable: 1
I took a look at this one, and it is a totally legitimate use, the
reason for which is explained with this comment:
/*
* check_stack_depth: check for excessively deep recursion
*
* This should be called so
Tom Lane wrote:
> quite a lot of people have looked at Postgres with Coverity
> already. If Clang is throwing up lots and lots of warnings, the
> odds are *very* high that most of them are false positives.
> Running through such a list to see if there's anything real isn't
> all that exciting a
Kevin Grittner writes:
> It does seem hard to believe that clang tools would find as enough
> problems that were missed by Coverity and Valgrind to account for
> all the warnings that are scrolling by; but it looks like it has
> pointed out at least *one* problem that's worth fixing.
Yeah, that's
On Mon, Nov 11, 2013 at 6:01 PM, Kevin Grittner wrote:
> Peter Geoghegan wrote:
>> Kevin Grittner wrote:
>>
>>> I'm currently capturing a text version of all the warnings from
>>> this. Will gzip and post when it finishes. It's generating a lot
>>> of warnings; I have no idea how many are Post
Peter Geoghegan wrote:
> Kevin Grittner wrote:
>
>> I'm currently capturing a text version of all the warnings from
>> this. Will gzip and post when it finishes. It's generating a lot
>> of warnings; I have no idea how many are PostgreSQL problems and
>> how many are false positives; will just
On Mon, Nov 11, 2013 at 5:51 PM, Peter Geoghegan wrote:
> On Mon, Nov 11, 2013 at 2:45 PM, Jeffrey Walton wrote:
>> I think you are right. Coverity is a very nice tool, and Clang has
>> some growing to do.
>
> To be fair to the LLVM/Clang guys, it's not as if static analysis is a
> very high prio
On Mon, Nov 11, 2013 at 5:18 PM, Kevin Grittner wrote:
> [moving the discussion to pgsql-hackers]
>
> Jeffrey Walton wrote:
>> ...
>> ##
>> # Sanitizers
>>
>> make distclean
>>
>> export DYLD_FALLBACK_LIBRARY_PATH=/usr/local/lib/clang/3.3/lib/darwin/
>> export CC=/usr/local/bin/clang
On Mon, Nov 11, 2013 at 2:45 PM, Jeffrey Walton wrote:
> I think you are right. Coverity is a very nice tool, and Clang has
> some growing to do.
To be fair to the LLVM/Clang guys, it's not as if static analysis is a
very high priority for them.
--
Peter Geoghegan
--
Sent via pgsql-hackers
On Mon, Nov 11, 2013 at 5:29 PM, Peter Geoghegan wrote:
> On Mon, Nov 11, 2013 at 2:18 PM, Kevin Grittner wrote:
>> I'm currently capturing a text version of all the warnings from
>> this. Will gzip and post when it finishes. It's generating a lot
>> of warnings; I have no idea how many are Pos
On Mon, Nov 11, 2013 at 2:18 PM, Kevin Grittner wrote:
> I'm currently capturing a text version of all the warnings from
> this. Will gzip and post when it finishes. It's generating a lot
> of warnings; I have no idea how many are PostgreSQL problems and
> how many are false positives; will just
[moving the discussion to pgsql-hackers]
Jeffrey Walton wrote:
> The Analyzer is invoked with scan-build. Its used when compiling
> the package because it performs static analysis.
>
> The Santizers are invoked with the runtime flags. They are used
> with the `check` program because they perform
35 matches
Mail list logo