Re: RFC: reduce CONFIG_SCSI_CONSTANTS impact by 4k

[Julian Calaby <julian.calaby@gmail.com>]

Hi Rasmus,

On Wed, Oct 7, 2015 at 2:39 AM, Rasmus Villemoes
<linux@rasmusvillemoes.dk> wrote:
> On Tue, Oct 06 2015, Julian Calaby <julian.calaby@gmail.com> wrote:
>
>> Hi Rasmus,
>>
>>>
>>> diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
>>> index 47aaccd5e68e..ccd34b0481cd 100644
>>> --- a/drivers/scsi/constants.c
>>> +++ b/drivers/scsi/constants.c
>>> @@ -292,17 +292,31 @@ bool scsi_opcode_sa_name(int opcode, int service_action,
>>>
>>>  struct error_info {
>>>         unsigned short code12;  /* 0x0302 looks better than 0x03,0x02 */
>>> -       const char * text;
>>> +       unsigned short size;
>>>  };
>>>
>>>
>>> +/*
>>> + * There are 700+ entries in this table. To save space, we don't store
>>> + * (code, pointer) pairs, which would make sizeof(struct
>>> + * error_info)==16 on 64 bits. Rather, the second element just stores
>>> + * the size (including \0) of the corresponding string, and we use the
>>> + * sum of these to get the appropriate offset into additional_text
>>> + * defined below. This approach saves 12 bytes per entry.
>>> + */
>>>  static const struct error_info additional[] =
>>>  {
>>> -#define SENSE_CODE(c, s) {c, s},
>>> +#define SENSE_CODE(c, s) {c, sizeof(s)},
>>>  #include "sense_codes.h"
>>>  #undef SENSE_CODE
>>>  };
>>>
>>> +static const char *additional_text =
>>> +#define SENSE_CODE(c, s) s "\0"
>>> +#include "sense_codes.h"
>>> +#undef SENSE_CODE
>>> +       ;
>>> +
>>>  struct error_info2 {
>>>         unsigned char code1, code2_min, code2_max;
>>>         const char * str;
>>> @@ -364,11 +378,14 @@ scsi_extd_sense_format(unsigned char asc, unsigned char ascq, const char **fmt)
>>>  {
>>>         int i;
>>>         unsigned short code = ((asc << 8) | ascq);
>>> +       unsigned offset = 0;
>>>
>>>         *fmt = NULL;
>>> -       for (i = 0; additional[i].text; i++)
>>> +       for (i = 0; i < ARRAY_SIZE(additional); i++) {
>>>                 if (additional[i].code12 == code)
>>> -                       return additional[i].text;
>>> +                       return additional_text + offset;
>>> +               offset += additional[i].size;
>>
>> You don't seem to be accounting for the null bytes here.
>
> Well, no, I account for the nul bytes where I define the table (the
> comment actually says as much). sizeof("foo") is 4. Since
> additional_text ends up pointing to a string containing
>
> "foo" "\0" "xyzzy" "\0" "..." "\0"
>
> aka
>
> "foo\0xyzzy\0...\0"
>
> this is the right amount to skip. As I said in the cover letter, I did
> test this (so that I'd at least catch silly off-by-ones), and I do get
> the right strings out.

Ah, that makes sense. It just didn't look right.

Thanks,

-- 
Julian Calaby

Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/