New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pointer.pointerTo... does not handle empty arrays #257

Closed
chinasaur opened this Issue Feb 7, 2012 · 4 comments

Comments

Projects
None yet
2 participants
@chinasaur

chinasaur commented Feb 7, 2012

Not too surprising that this causes problems, but maybe it could be addressed or else just documented?

I have a case of:
clbuffer.write(queue, Pointer.pointerToShorts(shortArr);

That I had to change for edge case to:
if (shortArr.length > 0) clbuffer.write(queue, Pointer.pointerToShorts(shortArr);

@chinasaur

This comment has been minimized.

Show comment
Hide comment
@chinasaur

chinasaur Feb 7, 2012

(JavaCL-1.0.0-RC1-shaded.jar)

chinasaur commented Feb 7, 2012

(JavaCL-1.0.0-RC1-shaded.jar)

@ochafik

This comment has been minimized.

Show comment
Hide comment
@ochafik

ochafik Feb 8, 2012

Member

Hi Peter,

Thanks for your report :-)
Pointer.pointerToXyx(new xyz[0]) is guaranteed to return null, since there is no such thing as a pointer to an empty array in C.
I believe it would be a bit weird to have CLBuffer.write(CLEvent, Pointer) to silently accept a null argument, wouldn't it be ?
What behaviour would you expect here ?

Cheers

Member

ochafik commented Feb 8, 2012

Hi Peter,

Thanks for your report :-)
Pointer.pointerToXyx(new xyz[0]) is guaranteed to return null, since there is no such thing as a pointer to an empty array in C.
I believe it would be a bit weird to have CLBuffer.write(CLEvent, Pointer) to silently accept a null argument, wouldn't it be ?
What behaviour would you expect here ?

Cheers

@chinasaur

This comment has been minimized.

Show comment
Hide comment
@chinasaur

chinasaur Feb 8, 2012

Hi, sorry about that; I thought I was getting an exception from pointerTo... but getting null there makes sense. If my exception is from CLBuffer#write, that makes more sense too, although I think accepting null silently and not writing anything would also make sense. You wouldn't want to change the behavior at this point though, so I think just documenting that null throws an exception would be nice.

Thanks!
P

chinasaur commented Feb 8, 2012

Hi, sorry about that; I thought I was getting an exception from pointerTo... but getting null there makes sense. If my exception is from CLBuffer#write, that makes more sense too, although I think accepting null silently and not writing anything would also make sense. You wouldn't want to change the behavior at this point though, so I think just documenting that null throws an exception would be nice.

Thanks!
P

ochafik added a commit that referenced this issue Feb 12, 2012

@ochafik

This comment has been minimized.

Show comment
Hide comment
@ochafik

ochafik Feb 12, 2012

Member

Hi @chinasaur,

I've added some checks to throw more informative IllegalArgumentException whenever input or output pointers are null in CLBuffer.read/write.
I agree this should translate into better javadocs as well, but there's just too much to do on that front, since JavaCL's javadoc is pretty empty right now (unlike BridJ's :-S). Any help in that direction would be highly welcome ;-)

Thanks again for your report,

Cheers

Member

ochafik commented Feb 12, 2012

Hi @chinasaur,

I've added some checks to throw more informative IllegalArgumentException whenever input or output pointers are null in CLBuffer.read/write.
I agree this should translate into better javadocs as well, but there's just too much to do on that front, since JavaCL's javadoc is pretty empty right now (unlike BridJ's :-S). Any help in that direction would be highly welcome ;-)

Thanks again for your report,

Cheers

@ochafik ochafik closed this Feb 12, 2012

ochafik added a commit that referenced this issue Feb 12, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment