#459 closed defect (fixed)
Vector Alloc, Realloc, Recycle
| Reported by: | 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 , 21 years ago
| Cc: | added |
|---|---|
| Owner: | changed from to |
comment:2 by , 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 , 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 , 21 years ago
| Cc: | added |
|---|
comment:5 by , 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 263 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 , 21 years ago
Actually, I was wondering why one uses PS specific-size types while the other
uses standard type.
comment:7 by , 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 , 21 years ago
| Resolution: | → fixed |
|---|---|
| Status: | new → closed |
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 , 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:11 by , 21 years ago
| op_sys: | Windows 2000 → All |
|---|---|
| Resolution: | fixed |
| Status: | closed → reopened |
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 , 21 years ago
| Cc: | 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 , 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:15 by , 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 , 21 years ago
| Owner: | changed from to |
|---|---|
| Status: | reopened → new |
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 , 21 years ago
| Resolution: | → fixed |
|---|---|
| Status: | new → closed |

Had already fixed this in the SDRS:
typedef struct {
} 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 {
} psImage;
psImage *psImageAlloc(psS32 numCols, psS32 numRows, psElemType type);
psImage* psImageRecycle(
image buffer
);
Not sure why image uses psS32 but vector uses "long". Josh?