Re: [PATCH 3/3]: Convert Reset code into socket error number

[Arnaldo Carvalho de Melo <acme@ghostprotocols.net>]

Em Tue, Oct 02, 2007 at 08:38:31AM +1300, Ian McDonald escreveu:
> On 9/30/07, Gerrit Renker <gerrit@erg.abdn.ac.uk> wrote:
> > [DCCP]: Convert Reset code into socket error number
> >
> > This adds support for converting the 11 currently defined Reset codes into system
> > error numbers, which are stored in sk_err for further interpretation.
> >
> > This makes the externally visible API behaviour similar to TCP, since a client
> > connecting to a non-existing port will experience ECONNREFUSED.
> >
> >  * Code 0, Unspecified, is interpreted as non-error (0);
> >  * Code 1, Closed (normal termination), also maps into 0;
> >  * Code 2, Aborted, maps into "Connection reset by peer" (ECONNRESET);
> >  * Code 3, No Connection and
> >    Code 7, Connection Refused, map into "Connection refused" (ECONNREFUSED);
> >  * Code 4, Packet Error, maps into "No message of desired type" (ENOMSG);
> >  * Code 5, Option Error, maps into "Illegal byte sequence" (EILSEQ);
> >  * Code 6, Mandatory Error, maps into "Operation not supported on transport endpoint" (EOPNOTSUPP);
> >  * Code 8, Bad Service Code, maps into "Invalid request code" (EBADRQC);
> >  * Code 9, Too Busy, maps into "Too many users" (EUSERS);
> >  * Code 10, Bad Init Cookie, maps into "Invalid request descriptor" (EBADR);
> >  * Code 11, Aggression Penalty, maps into "Quota exceeded" (EDQUOT)
> >    which makes sense in terms of using more than the `fair share' of bandwidth.
> >
> > The patch was found to solve the problem reported by Rémi Denis-Courmont - many thanks.
> >
> > Signed-off-by: Gerrit Renker <gerrit@erg.abdn.ac.uk>
> > ---
> >  net/dccp/input.c |   48 +++++++++++++++++++++++++++++++++++++++---------
> >  1 file changed, 39 insertions(+), 9 deletions(-)
> >
> > --- a/net/dccp/input.c
> > +++ b/net/dccp/input.c
> > @@ -55,6 +55,42 @@ static void dccp_rcv_closereq(struct soc
> >         dccp_send_close(sk, 0);
> >  }
> >
> > +static u8 dccp_reset_code_convert(const u8 code)
> > +{
> > +       const u8 error_code[] = {
> > +       [DCCP_RESET_CODE_CLOSED]             = 0,       /* normal termination */
> > +       [DCCP_RESET_CODE_UNSPECIFIED]        = 0,       /* nothing known */
> > +       [DCCP_RESET_CODE_ABORTED]            = ECONNRESET,
> > +
> > +       [DCCP_RESET_CODE_NO_CONNECTION]      = ECONNREFUSED,
> > +       [DCCP_RESET_CODE_CONNECTION_REFUSED] = ECONNREFUSED,
> > +       [DCCP_RESET_CODE_TOO_BUSY]           = EUSERS,
> > +       [DCCP_RESET_CODE_AGGRESSION_PENALTY] = EDQUOT,
> > +
> > +       [DCCP_RESET_CODE_PACKET_ERROR]       = ENOMSG,
> > +       [DCCP_RESET_CODE_BAD_INIT_COOKIE]    = EBADR,
> > +       [DCCP_RESET_CODE_BAD_SERVICE_CODE]   = EBADRQC,
> > +       [DCCP_RESET_CODE_OPTION_ERROR]       = EILSEQ,
> > +       [DCCP_RESET_CODE_MANDATORY_ERROR]    = EOPNOTSUPP,
> > +       };
> > +
> 
> This array is inside the function so is local.

Is that a problem?
 
> > +       return code <= DCCP_RESET_CODE_AGGRESSION_PENALTY? error_code[code] : 0;
> 
> and then you basically don't use half of it.

Not really, the code is correct, look at the definition of those codes:

enum dccp_reset_codes {
        DCCP_RESET_CODE_UNSPECIFIED = 0,
        DCCP_RESET_CODE_CLOSED,
        DCCP_RESET_CODE_ABORTED,
        DCCP_RESET_CODE_NO_CONNECTION,
        DCCP_RESET_CODE_PACKET_ERROR,
        DCCP_RESET_CODE_OPTION_ERROR,
        DCCP_RESET_CODE_MANDATORY_ERROR,
        DCCP_RESET_CODE_CONNECTION_REFUSED,
        DCCP_RESET_CODE_BAD_SERVICE_CODE,
        DCCP_RESET_CODE_TOO_BUSY,
        DCCP_RESET_CODE_BAD_INIT_COOKIE,
        DCCP_RESET_CODE_AGGRESSION_PENALTY,
};

He used a "designated initializer", i.e. he said at which index in the
array it the value is to be set. So it really doesn't matter the order.
And when checking if the code was within the valid range he tested
against the last entry in the enum. Correct code. :-)

- Arnaldo