Postgres Pro
Downloads
Home   >   mailing lists

Thread: BUG #8552: NEGATIVE_RETURNS in contrib/pageinspect/rawpage.c:83

BUG #8552: NEGATIVE_RETURNS in contrib/pageinspect/rawpage.c:83

From
jmlich@redhat.com
Date:
The following bug has been logged on the website:

Bug reference:      8552
Logged by:          Jozef Mlich
Email address:      jmlich@redhat.com
PostgreSQL version: 9.2.5
Operating system:   any
Description:

NEGATIVE_RETURNS in contrib/pageinspect/rawpage.c:83


Dear postgresql mainainers,


I was testing the postgresql using coverity scan tool. It reports following
problem:


1. postgresql-9.2.5/contrib/pageinspect/rawpage.c:81:negative_return_fn –
Function "forkname_to_number(text_to_cstring(forkname))" returns a negative
number.
15.
postgresql-9.2.5/src/backend/catalog/catalog.c:79:2:return_negative_constant
– Explicitly returning negative value "InvalidForkNumber".
16. postgresql-9.2.5/contrib/pageinspect/rawpage.c:81:var_assign –
Assigning: signed variable "forknum" = "forkname_to_number(char *)".
17. postgresql-9.2.5/contrib/pageinspect/rawpage.c:83:negative_returns –
Passing variable "forknum" to a parameter that cannot be negative.
30. postgresql-9.2.5/contrib/pageinspect/rawpage.c:146:2:index – Function
"ReadBufferExtended(Relation, ForkNumber, BlockNumber, ReadBufferMode,
BufferAccessStrategy)" uses "forknum" as an array index.
36. postgresql-9.2.5/src/backend/storage/buffer/bufmgr.c:248:2:index –
Function "ReadBuffer_common(SMgrRelation, char, ForkNumber, BlockNumber,
ReadBufferMode, BufferAccessStrategy, bool *)" uses "forkNum" as an array
index.
51. postgresql-9.2.5/src/backend/storage/buffer/bufmgr.c:377:4:index –
Function "relpathbackend(RelFileNode, BackendId, ForkNumber)" uses "forkNum"
as an array index.
54. postgresql-9.2.5/src/backend/catalog/catalog.c:131:4:index – Indexing
"forkNames" with "forknum".




In my opinion it worth to fix. I am not sure how to do it as I am not very
familiar with postgresql source code. I suggest to add assert into
relpathbackend in calalog.c or to add condition into rawpage.c. In case it
is not a bug, I would appreciate short explanation.


(1)
$ diff postgresql-9.2.5/src/backend/catalog/catalog.c.orig
postgresql-9.2.5/src/backend/catalog/catalog.c
122a123,124
> Assert(forknum == InvalidForkNumber);
>


(2)
$ diff postgresql-9.2.5/contrib/pageinspect/rawpage.c.orig
postgresql-9.2.5/contrib/pageinspect/rawpage.c
82a83,87
> if (forknum == InvalidForkNumber)
> {
> foo() // FIXME
> }
>




--
  regards,
  Jozef Mlich

Re: BUG #8552: NEGATIVE_RETURNS in contrib/pageinspect/rawpage.c:83

From
Haribabu kommi
Date:
T24gMjQgT2N0b2JlciAyMDEzIDE1OjExIGptbGljaCB3cm90ZToNCj5ORUdBVElWRV9SRVRVUk5T
IGluIGNvbnRyaWIvcGFnZWluc3BlY3QvcmF3cGFnZS5jOjgzDQo+DQo+RGVhciBwb3N0Z3Jlc3Fs
IG1haW5haW5lcnMsDQo+DQo+SSB3YXMgdGVzdGluZyB0aGUgcG9zdGdyZXNxbCB1c2luZyBjb3Zl
cml0eSBzY2FuIHRvb2wuIEl0IHJlcG9ydHMgZm9sbG93aW5nDQo+cHJvYmxlbToNCj4NCj4NCj4x
LiBwb3N0Z3Jlc3FsLTkuMi41L2NvbnRyaWIvcGFnZWluc3BlY3QvcmF3cGFnZS5jOjgxOm5lZ2F0
aXZlX3JldHVybl9mbiDigJMgRnVuY3Rpb24gImZvcmtuYW1lX3RvX251bWJlcih0ZXh0X3RvX2Nz
dHJpbmcoZm9ya25hbWUpKSIgcmV0dXJucyBhIG5lZ2F0aXZlIG51bWJlci4NCj4xNS4NCj5wb3N0
Z3Jlc3FsLTkuMi41L3NyYy9iYWNrZW5kL2NhdGFsb2cvY2F0YWxvZy5jOjc5OjI6cmV0dXJuX25l
Z2F0aXZlX2NvbnN0YW50DQo+4oCTIEV4cGxpY2l0bHkgcmV0dXJuaW5nIG5lZ2F0aXZlIHZhbHVl
ICJJbnZhbGlkRm9ya051bWJlciIuDQo+MTYuIHBvc3RncmVzcWwtOS4yLjUvY29udHJpYi9wYWdl
aW5zcGVjdC9yYXdwYWdlLmM6ODE6dmFyX2Fzc2lnbiDigJMNCj5Bc3NpZ25pbmc6IHNpZ25lZCB2
YXJpYWJsZSAiZm9ya251bSIgPSAiZm9ya25hbWVfdG9fbnVtYmVyKGNoYXIgKikiLg0KPjE3LiBw
b3N0Z3Jlc3FsLTkuMi41L2NvbnRyaWIvcGFnZWluc3BlY3QvcmF3cGFnZS5jOjgzOm5lZ2F0aXZl
X3JldHVybnMg4oCTIFBhc3NpbmcgdmFyaWFibGUgImZvcmtudW0iIHRvIGEgcGFyYW1ldGVyIHRo
YXQgY2Fubm90IGJlIG5lZ2F0aXZlLg0KPjMwLiBwb3N0Z3Jlc3FsLTkuMi41L2NvbnRyaWIvcGFn
ZWluc3BlY3QvcmF3cGFnZS5jOjE0NjoyOmluZGV4IOKAkyBGdW5jdGlvbiAiUmVhZEJ1ZmZlckV4
dGVuZGVkKFJlbGF0aW9uLCBGb3JrTnVtYmVyLCBCbG9ja051bWJlciwgUmVhZEJ1ZmZlck1vZGUs
IEJ1ZmZlckFjY2Vzc1N0cmF0ZWd5KSIgdXNlcyAiZm9ya251bSIgYXMgYW4gYXJyYXkgaW5kZXgu
DQozNi4gcG9zdGdyZXNxbC05LjIuNS9zcmMvYmFja2VuZC9zdG9yYWdlL2J1ZmZlci9idWZtZ3Iu
YzoyNDg6MjppbmRleCDigJMgRnVuY3Rpb24gIlJlYWRCdWZmZXJfY29tbW9uKFNNZ3JSZWxhdGlv
biwgY2hhciwgRm9ya051bWJlciwgQmxvY2tOdW1iZXIsIFJlYWRCdWZmZXJNb2RlLCBCdWZmZXJB
Y2Nlc3NTdHJhdGVneSwgYm9vbCAqKSIgdXNlcyANCj4iZm9ya051bSIgYXMgYW4gYXJyYXkgaW5k
ZXguDQo+NTEuIHBvc3RncmVzcWwtOS4yLjUvc3JjL2JhY2tlbmQvc3RvcmFnZS9idWZmZXIvYnVm
bWdyLmM6Mzc3OjQ6aW5kZXgg4oCTIEZ1bmN0aW9uICJyZWxwYXRoYmFja2VuZChSZWxGaWxlTm9k
ZSwgQmFja2VuZElkLCBGb3JrTnVtYmVyKSIgdXNlcyAiZm9ya051bSINCj5hcyBhbiBhcnJheSBp
bmRleC4NCj41NC4gcG9zdGdyZXNxbC05LjIuNS9zcmMvYmFja2VuZC9jYXRhbG9nL2NhdGFsb2cu
YzoxMzE6NDppbmRleCDigJMgSW5kZXhpbmcgImZvcmtOYW1lcyIgd2l0aCAiZm9ya251bSIuDQo+
DQo+SW4gbXkgb3BpbmlvbiBpdCB3b3J0aCB0byBmaXguIEkgYW0gbm90IHN1cmUgaG93IHRvIGRv
IGl0IGFzIEkgYW0gbm90IHZlcnkgZmFtaWxpYXIgd2l0aCBwb3N0Z3Jlc3FsIHNvdXJjZSBjb2Rl
LiBJIHN1Z2dlc3QgdG8gYWRkIGFzc2VydCBpbnRvIHJlbHBhdGhiYWNrZW5kIGluIGNhbGFsb2cu
YyBvciB0byBhZGQgY29uZGl0aW9uIGludG8gDQo+cmF3cGFnZS5jLiBJbiBjYXNlIGl0IGlzIG5v
dCBhIGJ1ZywgSSB3b3VsZCBhcHByZWNpYXRlIHNob3J0IGV4cGxhbmF0aW9uLg0KDQpUaGUgZnVu
Y3Rpb24gZm9ya25hbWVfdG9fbnVtYmVyKCkgZG9lc24ndCByZXR1cm4gSW52YWxpZEZvcmtOdW1i
ZXIuIEJlZm9yZSB0aGUgcmV0dXJuIHN0YXRlbWVudCB0aGVyZSBpcyBhbiBlcmVwb3J0IGlzIHBy
ZXNlbnQuDQpUaGUgcmV0dXJuIHN0YXRlbWVudCBpcyB1c2VkIGZvciBrZWVwaW5nIHRoZSBjb21w
aWxlciBxdWlldC4gVGhlIGNvdmVyaXR5IHRvb2wgaXMgcmVwb3J0aW5nIHRoZSBlcnJvcnMgYXMg
aXQgaXMgbm90IGRldGVjdGluZyB0aGUgZXJlcG9ydC4NCg0KUmVnYXJkcywNCkhhcmkgQmFidS4N
Cg0K