Re: [PATCH] s390: Hypervisor File System

[Michael Holzheu]

Hi Joern,

Jörn Engel <joern@xxxxxxxxxxxxxxxxxxxx> wrote on 04/28/2006 07:43:02 PM:
> On Fri, 28 April 2006 19:36:30 +0200, Michael Holzheu wrote:
> > Andrew Morton <akpm@xxxxxxxx> wrote on 04/28/2006 11:56:21 AM:
> > > Michael Holzheu <holzheu@xxxxxxxxxx> wrote:
> >
> > > > +static int diag224_idx2name(int index, char *name)
> > > > +{
> > > > +   memcpy(name, diag224_cpu_names + ((index + 1) * 16), 16);
> > > > +   name[16] = 0;
> > >
> > > Should this be "15"?   I guess not...
> >
> > No bug, our strings here have 16 characters and are not
> > 0 terminated.
>
> Hmm.  TMP_SIZE is defined to 64 and used for buffers allocated on the
> stack.  It is not too excessive, but in this case 17 would definitely
> be enough.  Not sure if it's worth going through.
>
> Jörn

Since I use the buffers with size TMP_SIZE also for other purposes
in the same functions, where the cpu types are accessed,
I think, it is not useful to define a second buffer with size 17.

But I think it is better to have a define for the buffer size
of cpu names. something like:

 #define LPAR_NAME_LEN 8                /* lpar name len in diag 204 data
*/
+#define CPU_NAME_LEN 16                /* type name len of a cpu in
diag224 name table */
 #define TMP_SIZE 64            /* size of temporary buffers */

 static int diag224_idx2name(int index, char *name)
 {
-       memcpy(name, diag224_cpu_names + ((index + 1) * 16), 16);
-       name[16] = 0;
+       memcpy(name, diag224_cpu_names + ((index + 1) * CPU_NAME_LEN),
+               CPU_NAME_LEN);
+       name[CPU_NAME_LEN] = 0;
        strstrip(name);
        return 0;
 }

Michael

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/