IPP Software Navigation Tools IPP Links Communication Pan-STARRS Links

Opened 21 years ago

Closed 21 years ago

Last modified 21 years ago

#459 closed defect (fixed)

Vector Alloc, Realloc, Recycle

Reported by: David.Robbins@… Owned by: Paul Price
Priority: high Milestone:
Component: PSLib SDRS Version: unspecified
Severity: normal Keywords:
Cc: rhl@…

Description

Arguments should actually take 'unsigned long' as opposed to 'long'. Same
argument as before for ArrayAlloc, Realloc, Get, Set.

Change History (17)

comment:1 by Paul Price, 21 years ago

Cc: jhoblitt@… added
Owner: changed from Paul Price to jhoblitt

Had already fixed this in the SDRS:

typedef struct {

psMathType type; /< vector data type and dimension
long n;
/< size of vector
const long nalloc; /< allocated data block
union {

psS8 *S8; /< Pointers to byte data
psS16 *S16;
/< Pointers to short-integer data
psS32 *S32; /< Pointers to integer data
psS64 *S64;
/< Pointers to long-integer data
psU8 *U8; /< Pointers to unsigned-byte data
psU16 *U16;
/< Pointers to unsigned-short-integer data
psU32 *U32; /< Pointers to unsigned-integer data
psU64 *U64;
/< Pointers to unsigned-long-integer data
psF32 *F32; /< Pointers to floating-point data
psF64 *F64;
/< Pointers to double-precision data
psC32 *C32; /< Pointers to complex floating-point data
psC64 *C64;
/< Pointers to complex floating-point data

} data;
void *lock; /< Lock for thread safety

} psVector;

psVector *psVectorAlloc(long nalloc, psElemType type);
psVector *psVectorRealloc(psVector *vector, long nalloc);
psVector *psVectorRecycle(psVector *vector, long nalloc, psElemType type);

and for psImage:

typedef struct psImage {

psMathType type; /< image data type and dimension
const psS32 numCols;
/< Number of columns in image
const psS32 numRows; /< Number of rows in image.
const psS32 col0;
/< Column position relative to parent.
const psS32 row0; /< Row position relative to parent.

union {

psS8 S8; /< Pointers to char data
psS16
S16; /< Pointers to short-integer data
psS32 S32; /< Pointers to integer data
psS64
S64; /< Pointers to long-integer data
psU8 U8; /< Pointers to unsigned-char data
psU16
U16; /< Pointers to unsigned-short-integer data
psU32 U32; /< Pointers to unsigned-integer data
psU64
U64; /< Pointers to unsigned-long-integer data
psF32 F32; /< Pointers to floating-point data
psF64
F64; /< Pointers to double-precision data
psC32 C32; /< Pointers to complex floating-point data
psC64
C64; /< Pointers to complex floating-point data
psPtr V; /< Pointers to raw data

} data;
const struct psImage *parent; /< parent, if a subimage
psArray *children;
/< children of this region
void *lock; /< Lock for thread safety

} psImage;

psImage *psImageAlloc(psS32 numCols, psS32 numRows, psElemType type);
psImage* psImageRecycle(

psImage* old, /< the psImage to recycle by resizing

image buffer

psS32 numCols, /< the desired number of columns in image
psS32 numRows,
/< the desired number of rows in image
const psElemType type /< the desired datatype of the image

);

Not sure why image uses psS32 but vector uses "long". Josh?

comment:2 by David.Robbins@…, 21 years ago

Sorry, I was referring to the nalloc argument, ie.,
current:
psVector *psVectorAlloc(long nalloc, psElemType type);

should be:
psVector *psVectorAlloc(unsigned long nalloc, psElemType type);

comment:3 by jhoblitt, 21 years ago

My thinking at the time was a 32bit 2D array can address approximate the same
number of elements as a 64bit 1D array can address (~1.8e19). So psImage can
already hold an image larger then the ammount of ram we'll be able to purchase
in a single system in the next decade. Does psImage really need to support 2
billion2 pixel arrays?

comment:4 by jhoblitt, 21 years ago

Cc: price@… added

comment:5 by jhoblitt, 21 years ago

We've been having some debate about signed vs. unsigned allocators recently. My
position on this is that since we want to be able to index a psVector from the
end of the array it is rather unconvient if someone can allocate a 263+ sized
vector but only be able to address the first 2
63 bits with positive integers.
So I beleive it makes since to keep nalloc signed and to check for error on
negativie values. It's 'unlikely' that we'll want to allocate a vector that
large anyways...

comment:6 by Paul Price, 21 years ago

Actually, I was wondering why one uses PS specific-size types while the other
uses standard type.

comment:7 by jhoblitt, 21 years ago

I guess in the case of psImage we're trying to say this value should always be
32 bits where in the case of psVector we're trying to say this value can be 32
bits or 64 bits. It's probably safe to use ints in psImage...

comment:8 by Paul Price, 21 years ago

Resolution: fixed
Status: newclosed

OK, changed:

typedef struct psImage {

psMathType type; /< image data type and dimension
const int numCols;
/< Number of columns in image
const int numRows; /< Number of rows in image.
const int col0;
/< Column position relative to parent.
const int row0; /< Row position relative to parent.
union {

psS8 S8; /< Pointers to char data
psS16
S16; /< Pointers to short-integer data
psS32 S32; /< Pointers to integer data
psS64
S64; /< Pointers to long-integer data
psU8 U8; /< Pointers to unsigned-char data
psU16
U16; /< Pointers to unsigned-short-integer data
psU32 U32; /< Pointers to unsigned-integer data
psU64
U64; /< Pointers to unsigned-long-integer data
psF32 F32; /< Pointers to floating-point data
psF64
F64; /< Pointers to double-precision data
psC32 C32; /< Pointers to complex floating-point data
psC64
C64; /< Pointers to complex floating-point data
psPtr V; /< Pointers to raw data

} data;
const struct psImage *parent; /< parent, if a subimage
psArray *children;
/< children of this region
void *lock; /< Lock for thread safety

} psImage;

psImage *psImageAlloc(int numCols, int numRows, psElemType type);
psImage* psImageRecycle(

psImage* old, /< the psImage to recycle by resizing

image buffer

int numCols, /< the desired number of columns in image
int numRows,
/< the desired number of rows in image
const psElemType type /< the desired datatype of the image

);

comment:9 by David.Robbins@…, 21 years ago

So, just to verify, you've chosen to use signed int's in psImage and psVector?
This seems inconsistent to me so I'm just double-checking before making these
changes.
(If they are signed, perhaps you could explain how the negative values are used
if you have time?)

comment:10 by jhoblitt, 21 years ago

Please read comment #4 about indexing from the end of the array.

comment:11 by robert.desonia@…, 21 years ago

op_sys: Windows 2000All
Resolution: fixed
Status: closedreopened

I think the question is not "do we need to access that many values", but a
matter of consistency. If we are to say allocations sizes are unsigned, then
that should be universally applied in the API, and specifically here.

I would vote for 'size_t', myself.

BTW: is indexing really limited to sign values in C? Comment #4 really doesn't
make much sense to me otherwise.

-rdd

comment:12 by jhoblitt, 21 years ago

Cc: rhl@… added

You are proposing that indexing from the tail of an array/vector/etc. is done by
arthimetic negation of an unsigned type. I have no idea if this behavior is
guarenteed. If you can point me to an ANSI, IEEE, etc. standard that
*guarantees* this as portable behavior I will concede to using unsigned 'nalloc'
types.

negation done by the compiler
--
#include <limits.h>

int main ()
{

unsigned int index;

the position of the 2nd element in the 'array' counting from the tail.
index = -UINT_MAX;

is this *guaranteed* to be 1?
printf("%u\n", index);

}
--

negation done at runtime
--
#include <limits.h>

int main ()
{

unsigned int index;
volatile unsigned int tmp;

the position of the 2nd element in the 'array' counting from the tail.
tmp = UINT_MAX;
index = - tmp;

guaranteed to be 1?
printf("%u\n", index);

}
--

comment:13 by robert.desonia@…, 21 years ago

Let's not go off on the tangent trivia of indexing in C and just cover the
consistency in the API issue.

comment:14 by jhoblitt, 21 years ago

In that case, signed types are consistent with the indexing scheme.

comment:15 by robert.desonia@…, 21 years ago

But it is not really consistent with the API where allocation sizes are
unsigned, as is the case in psAlloc.

psVectorAlloc is not the only function that allows a signed allocation size
either. We just a week or so ago pointed out psArrayAlloc, psVectorAlloc's
little brother, and the answer was to make nalloc unsigned.

I think it would be best to (re)consider when unsigned is to be used, and when
not used, on a global basis in the API. Personnally, It really makes little
sense to have a negative number of elements allocated.

-rdd

comment:16 by Paul Price, 21 years ago

Owner: changed from jhoblitt to Paul Price
Status: reopenednew

I've cleaned up the SDRS following this morning's conversation:

\paragraph{Negative allocations}

Note that we have specified that the memory size is unsigned
(\code{size_t}), so that we can address the full range of memory that
the architecture will allow, and to match the behaviour of the system
\code{malloc}. This creates the potential for problems if a negative
value is inadvertently passed to \code{psAlloc} --- it will be
interpreted as a very large positive value. To guard against this, we
specify that \code{psAlloc} must check that the allocation is less
than \code{PS_MEM_LIMIT} (a preprocessor variable).

For array-like collections (specifically, \code{psArray},
\code{psPixels}, \code{psVector}, and \code{psImage}) we allow the
user to refer to a negative index in the accessor (e.g.,
\code{psArrayGet}) to mean address from the end. Consequently, the
number of elements in structures should be signed (in order to be able
to access the full range of allocated values). It is the
responsibility of these structure allocators (e.g.,
\code{psArrayAlloc}) to check that the requested number of elements is
not negative (calling \code{psAbort} otherwise). All other allocators
shall simply use \code{size_t} where the number of elements is needed
(saving the trouble of checking before passing to psAlloc).

...

bool psArraySet(psArray *array, long position, psPtr data);
psPtr psArrayGet(const psArray *array, long position);

bool psPixelsSet(psPixels *pixels, long position, psPixelCoord value);
psPixelCoord psPixelsGet(const psPixels *pixels, long position);

bool psVectorSet(const psVector *input, long position, complex double value);
complex double psVectorGet(const psVector *input, long position);

bool psImageSet(const psImage *image, int x, int y, complex double value);
complex double psImageGet(const psImage *image, int x, int y);

The blurb for each of these sets is pretty much the same:

These accessor functions are provided as a convenience to the user.
\code{psImageSet} sets the value of the \code{image} at the specified
\code{x,y} position to \code{value} (appropriately cast), returning
\code{true} if successful. \code{psImageGet} returns the value of the
\code{image} at the specified \code{x,y} position. A negative value
for the \code{x,y} position means index from the end.

comment:17 by Paul Price, 21 years ago

Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.