Skip to content
This repository has been archived by the owner on Dec 9, 2018. It is now read-only.

Cgo drnm2 panics with zero-length slice #133

Closed
btracey opened this issue Aug 24, 2015 · 8 comments
Closed

Cgo drnm2 panics with zero-length slice #133

btracey opened this issue Aug 24, 2015 · 8 comments

Comments

@btracey
Copy link
Member

btracey commented Aug 24, 2015

Dnrm2 allows a zero-length slice, but cgo does not check for zero length before taking &x[0].

@kortschak
Copy link
Member

This will be true of all the functions that take an n and a vector. I can add a check for n == 0 before the cgo call and return early then. If the ret type is int, return -1; this catches I*amax. The native implementations have some potential panics prior to the equivalent test, so presumably I should mimic that, but what is the basis for those?

@btracey
Copy link
Member Author

btracey commented Aug 24, 2015

I'm not sure what you mean by "the basis for those". In Idamax right now the code is:

if n < 2 {
        if n == 1 {
            return math.Abs(x[0])
        }
        if n == 0 {
            return 0
        }
        if n < 1 {
            panic(negativeN)
        }
    }

It returns 0 and not -1 because the refernce BLAS returns 1 when the size is zero.

@btracey
Copy link
Member Author

btracey commented Aug 24, 2015

Sorry, BLAS returns 1 for Idamax when the slice length is zero, and also returns 0 when the slice is zero for Dnrm2.

@btracey
Copy link
Member Author

btracey commented Aug 24, 2015

Easy to change if we'd rather it return -1. As far as I'm concerned, as long as we're doing something reasonable and cgo and native have the same behavior, I'm happy to deviate from the strict blas standard.

@kortschak
Copy link
Member

There are panics for incX < 0 when n == 0.

@btracey
Copy link
Member Author

btracey commented Aug 24, 2015

Yes.

Also, sorry, I misread the code. LAPACK does return an invalid index when n == 0.

@kortschak
Copy link
Member

OK, so what would you like. I have the hooks for int, float and none - in the float64 case we return 0, for int?

(What on earth is the basis for returning 1 when n == 0?).

@btracey
Copy link
Member Author

btracey commented Aug 24, 2015

I think Idamax should return -1. This is consistent with the current behavior and the reference standard. (Reference returns 0, but it's one-indexed)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants