Re: Correct error number in crypto(9)

2016-02-27 Thread Michael McConville
Mike Belopuhov wrote:
> On 27 February 2016 at 08:21, Michael McConville  wrote:
> > Michael McConville wrote:
> >> Michael McConville wrote:
> >> > Does this make sense?
> >>
> >> I just realized that the allocation failure checks earlier in the
> >> function return ENOBUFS. This probably makes more sense for the sake of
> >> consistency.
> >
> > The best I can tell, the only use of this function is in
> > sys/crypto/crypto.c:157. It's accessed through a pointer stored in a
> > struct by crypto_register(). That usage doesn't seem to be affected
> > by the below change, considering that the outcome would be no
> > different than that of the other ENOBUFS failures above it.
> >
> 
> So why change it to ENOMEM then?  Nothing there returns it.
> I think this is just needless churn.

I was proposing to change it from EINVAL to ENOBUFS.

I don't think it's churn. The current returned error code seems
objectively wrong to me, and all other allocation failures in this
function return ENOBUFS. deraadt supported it but asked me to ensure
that it wouldn't affect downstream error handling.



Re: Correct error number in crypto(9)

2016-02-27 Thread Mike Belopuhov
On 27 February 2016 at 08:21, Michael McConville  wrote:
> Michael McConville wrote:
>> Michael McConville wrote:
>> > Does this make sense?
>>
>> I just realized that the allocation failure checks earlier in the
>> function return ENOBUFS. This probably makes more sense for the sake of
>> consistency.
>
> The best I can tell, the only use of this function is in
> sys/crypto/crypto.c:157. It's accessed through a pointer stored in a
> struct by crypto_register(). That usage doesn't seem to be affected by
> the below change, considering that the outcome would be no different
> than that of the other ENOBUFS failures above it.
>

So why change it to ENOMEM then?  Nothing there returns it.
I think this is just needless churn.



Re: Correct error number in crypto(9)

2016-02-26 Thread Michael McConville
Michael McConville wrote:
> Michael McConville wrote:
> > Does this make sense?
> 
> I just realized that the allocation failure checks earlier in the
> function return ENOBUFS. This probably makes more sense for the sake of
> consistency.

The best I can tell, the only use of this function is in
sys/crypto/crypto.c:157. It's accessed through a pointer stored in a
struct by crypto_register(). That usage doesn't seem to be affected by
the below change, considering that the outcome would be no different
than that of the other ENOBUFS failures above it.

> > Index: sys/crypto/cryptosoft.c
> > ===
> > RCS file: /cvs/src/sys/crypto/cryptosoft.c,v
> > retrieving revision 1.80
> > diff -u -p -r1.80 cryptosoft.c
> > --- sys/crypto/cryptosoft.c 10 Dec 2015 21:00:51 -  1.80
> > +++ sys/crypto/cryptosoft.c 26 Feb 2016 17:21:00 -
> > @@ -826,7 +826,7 @@ swcr_newsession(u_int32_t *sid, struct c
> > M_CRYPTO_DATA, M_NOWAIT | M_ZERO);
> > if ((*swd)->sw_kschedule == NULL) {
> > swcr_freesession(i);
> > -   return EINVAL;
> > +   return ENOMEM;
> > }
> > }
> > if (txf->setkey((*swd)->sw_kschedule, cri->cri_key,
> > 
> 



Re: Correct error number in crypto(9)

2016-02-26 Thread Michael McConville
Michael McConville wrote:
> Does this make sense?

I just realized that the allocation failure checks earlier in the
function return ENOBUFS. This probably makes more sense for the sake of
consistency.

> Index: sys/crypto/cryptosoft.c
> ===
> RCS file: /cvs/src/sys/crypto/cryptosoft.c,v
> retrieving revision 1.80
> diff -u -p -r1.80 cryptosoft.c
> --- sys/crypto/cryptosoft.c   10 Dec 2015 21:00:51 -  1.80
> +++ sys/crypto/cryptosoft.c   26 Feb 2016 17:21:00 -
> @@ -826,7 +826,7 @@ swcr_newsession(u_int32_t *sid, struct c
>   M_CRYPTO_DATA, M_NOWAIT | M_ZERO);
>   if ((*swd)->sw_kschedule == NULL) {
>   swcr_freesession(i);
> - return EINVAL;
> + return ENOMEM;
>   }
>   }
>   if (txf->setkey((*swd)->sw_kschedule, cri->cri_key,
> 



Correct error number in crypto(9)

2016-02-26 Thread Michael McConville
Does this make sense?


Index: sys/crypto/cryptosoft.c
===
RCS file: /cvs/src/sys/crypto/cryptosoft.c,v
retrieving revision 1.80
diff -u -p -r1.80 cryptosoft.c
--- sys/crypto/cryptosoft.c 10 Dec 2015 21:00:51 -  1.80
+++ sys/crypto/cryptosoft.c 26 Feb 2016 17:21:00 -
@@ -826,7 +826,7 @@ swcr_newsession(u_int32_t *sid, struct c
M_CRYPTO_DATA, M_NOWAIT | M_ZERO);
if ((*swd)->sw_kschedule == NULL) {
swcr_freesession(i);
-   return EINVAL;
+   return ENOMEM;
}
}
if (txf->setkey((*swd)->sw_kschedule, cri->cri_key,