Re: [patch] [SCSI] libosd: check for kzalloc() failure

[Boaz Harrosh <bharrosh@panasas.com>]

On 01/30/2013 04:34 PM, walter harms wrote:
> 
<>
> I start to see the complexity of the situation. Would you mind to add
> the comment "it can be anything.  UTF-8 is more likely but not guaranteed either" ?
> For now using a pascal-string seems the best solution but it should be warned
> that get_attrs[a].val_ptr is NOT a c-string and therefore can not be used with
> the printf-family (i guess the situation will become more clear in future)
> 

OK, So... The long story

The STD says that osdname can be *anything* binary or otherwise, and of *any*
length. And is used to uniquely identify the OSD-device/partition in a cluster.

We decided that if so we would stick an 40 char UUID in it, generated by a uuidgen
and all the external utilities and surrounding code forces it, and treat it as
a c-string. But this code here in the core cannot make that assumption and still
support the STD.

On the other hand we did want the osdname to be printable in traces and messages
because it is such an important identifier. So I have decided to sacrifice an
extra char in-memory to carry a \NUL and safely stick it into printk(s). Those
(none existent) OSD devices that will put unprintable characters in here will
still function fine, but will look real scary in printk(s).

Please note that the one that sets policy is the osd-target vendor. (And they
all currently use my code base)

So save the kzalloc check this code (tested) is safe and will show strings when
present, but will gracefully show ugly things but still work when not.

> I have no clue why you need that, using c-strings makes life more easy in the
> last minute a sprintf(buf,"%u") will save the day ;)
> 

Actually it is very funny, because just recently (last week) I have discovered
something that eliminates all those funny business.

	printf("%*s", odi->osdname, odi->osdname_len);

The "*" will instruct c to expect an extra variable following %s which is the
max_length of %s. This is exactly for pascal strings and the such like above.

So I added a TODO to clean that a bit by always printing with "%*s"

>>
> It look clever to add the NULL test here.
> Noone reviewing the code will understand that.
> (Rule of least surprise)
> 

Thanks for looking, I agree it needs a fat comment, I'll do that when I'll
convert to above system. Thanks for looking, That code is really old and never
had any extra eyes on it.

> just my 2 cents,
> re,
>  wh
> 

Cheers
Boaz