Thread: use of int4/int32 in C code
What is the latest theory on using int4 vs. int32 in C code? (equivalently int2, int16) I had the idea that using int4 was sort of deprecated, and most code uses int32, but I've come across several uses of int4 lately that looked odd to me. I think the main reason that we define int4 in C is for the src/include/catalog/ files, but that won't work anymore if we ever want to have an int8 column there. Ideas: * Leave it be. * Replace int4 by int32 everywhere except in the catalog files. Hope it stays that way. * Mark int4 as deprecated, change catalog files to use int32, adjust the bki generation scripts. (I think removing int4 might not be wise, as external modules might be using it.) While we're at it, how do we feel about using C standard types like int32_t instead of (or initially in addition to) our own definitions? These are well established by now and would help modernize our code and the code of extensions a bit.
Peter Eisentraut <peter_e@gmx.net> writes: > What is the latest theory on using int4 vs. int32 in C code? > (equivalently int2, int16) I thought the general idea was to use int32 most places, but int4 in catalog declarations. I don't think it's tremendously important if somebody uses the other though. > While we're at it, how do we feel about using C standard types like > int32_t instead of (or initially in addition to) our own definitions? Can't get very excited about this either. The most likely outcome of a campaign to substitute the standard types is that back-patching would become a truly painful activity. IMO, anything that is going to result in tens of thousands of diffs had better have a more-than-cosmetic reason. (That wouldn't apply if we only used int32_t in new code ... but then, instead of two approved ways to do it, there would be three. Which doesn't seem like it improves matters.) regards, tom lane
On Tue, Jun 19, 2012 at 9:47 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Peter Eisentraut <peter_e@gmx.net> writes: >> What is the latest theory on using int4 vs. int32 in C code? >> (equivalently int2, int16) > > I thought the general idea was to use int32 most places, but int4 in > catalog declarations. I don't think it's tremendously important if > somebody uses the other though. I concur with Peter that TMTOWTDI is not the right way to do this. I think we ought to get rid of int4 in code and use int32 everywhere. >> While we're at it, how do we feel about using C standard types like >> int32_t instead of (or initially in addition to) our own definitions? > > Can't get very excited about this either. The most likely outcome of > a campaign to substitute the standard types is that back-patching would > become a truly painful activity. IMO, anything that is going to result > in tens of thousands of diffs had better have a more-than-cosmetic > reason. (That wouldn't apply if we only used int32_t in new code ... > but then, instead of two approved ways to do it, there would be three. > Which doesn't seem like it improves matters.) On this one, I agree with you. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On 19 June 2012 20:11, Robert Haas <robertmhaas@gmail.com> wrote: > On Tue, Jun 19, 2012 at 9:47 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> Peter Eisentraut <peter_e@gmx.net> writes: >>> What is the latest theory on using int4 vs. int32 in C code? >>> (equivalently int2, int16) >> >> I thought the general idea was to use int32 most places, but int4 in >> catalog declarations. I don't think it's tremendously important if >> somebody uses the other though. > > I concur with Peter that TMTOWTDI is not the right way to do this. I > think we ought to get rid of int4 in code and use int32 everywhere. > >>> While we're at it, how do we feel about using C standard types like >>> int32_t instead of (or initially in addition to) our own definitions? >> >> Can't get very excited about this either. The most likely outcome of >> a campaign to substitute the standard types is that back-patching would >> become a truly painful activity. IMO, anything that is going to result >> in tens of thousands of diffs had better have a more-than-cosmetic >> reason. (That wouldn't apply if we only used int32_t in new code ... >> but then, instead of two approved ways to do it, there would be three. >> Which doesn't seem like it improves matters.) > > On this one, I agree with you. Yeah. I find pgindent changes annoying when doing a git blame myself. Now, granted, you can mostly take care of that by having the tool ignore whitespace changes, but that doesn't always work perfectly, and I haven't been able to figure out a better way of managing that. -- Peter Geoghegan http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training and Services
Robert Haas <robertmhaas@gmail.com> writes: > On Tue, Jun 19, 2012 at 9:47 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> I thought the general idea was to use int32 most places, but int4 in >> catalog declarations. I don't think it's tremendously important if >> somebody uses the other though. > I concur with Peter that TMTOWTDI is not the right way to do this. I > think we ought to get rid of int4 in code and use int32 everywhere. I have not looked to see how many places do that. If it's a reasonably small number of places, I'm OK with getting rid of int4 at the C level. (int2/int8 the same of course.) If we are going to do that, though, we need to actually remove those typedefs. Leaving them around on the grounds that third-party code might be using them will just allow cases to creep back in. regards, tom lane
Tom Lane <tgl@sss.pgh.pa.us> wrote: > I have not looked to see how many places do that. If it's a reasonably > small number of places, I'm OK with getting rid of int4 at the C level. > (int2/int8 the same of course.) $ find -name '*.h' -or -name '*.c' | egrep -v '/tmp_check/' | xargs cat \ | egrep -c '\bint2\b' 178 | egrep -c '\bint4\b' 570 | egrep -c '\bint8\b' 212 -Kevin
"Kevin Grittner" <Kevin.Grittner@wicourts.gov> writes: > Tom Lane <tgl@sss.pgh.pa.us> wrote: >> I have not looked to see how many places do that. If it's a reasonably >> small number of places, I'm OK with getting rid of int4 at the C level. >> (int2/int8 the same of course.) > $ find -name '*.h' -or -name '*.c' | egrep -v '/tmp_check/' | xargs cat > | egrep -c '\bint4\b' > 570 That one looks like a lot, but a quick eyeball check suggests that many of them are in comments and/or embedded SQL fragments where they actually represent references to the SQL datatype, and so should be left alone. The impression I get is that the actual typedef uses are concentrated in a couple of contrib modules and some of the tsearch stuff, where they'd probably not be that painful to fix. regards, tom lane