[KJ] drivers\char\synclink.c

[KJ] drivers\char\synclink.c

[Christophe Jaillet]

[-- Attachment #1.1: Type: text/plain, Size: 407 bytes --]

In file :            \drivers\char\synclink.c
In function :    line_info(char *buf, struct mgsl_struct *info)

The buffer *stat_buf* could be defined smaller.

It is used to build a string that can not be longer than "|RTS|CTS|DTR|DSR|CD|RI", that is to say 21 + 1 chars so :

     char stat_buf[22];
instead of
    char stat_buf[30];
would be enough and would save a few bytes on the stack.

CJ

[-- Attachment #1.2: Type: text/html, Size: 1395 bytes --]

[-- Attachment #2: Type: text/plain, Size: 168 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] drivers\char\synclink.c

[Nish Aravamudan]

On 11/25/05, Christophe Jaillet <christophe.jaillet@wanadoo.fr> wrote:
>
> In file :            \drivers\char\synclink.c
> In function :    line_info(char *buf, struct mgsl_struct *info)
>
> The buffer *stat_buf* could be defined smaller.


Where's the patch?

Thanks,
Nish

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] drivers\char\synclink.c

[Matthew Wilcox]

[-- Attachment #1: Type: text/plain, Size: 601 bytes --]

On Fri, Nov 25, 2005 at 01:34:03PM -0800, Nish Aravamudan wrote:
> On 11/25/05, Christophe Jaillet <christophe.jaillet@wanadoo.fr> wrote:
> >
> > In file :            \drivers\char\synclink.c
> > In function :    line_info(char *buf, struct mgsl_struct *info)
> >
> > The buffer *stat_buf* could be defined smaller.
> 
> 
> Where's the patch?

Never mind that.  Is this a patch we even want?  It's quite common to
overallocate these small string buffers so that when someone makes a
change and forgets to update the length, they don't overrun the buffer.

And what's with the backward slashes anyway?

[-- Attachment #2: Type: text/plain, Size: 168 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] drivers\char\synclink.c

[Nish Aravamudan]

On 11/25/05, Matthew Wilcox <matthew@wil.cx> wrote:
> On Fri, Nov 25, 2005 at 01:34:03PM -0800, Nish Aravamudan wrote:
> > On 11/25/05, Christophe Jaillet <christophe.jaillet@wanadoo.fr> wrote:
> > >
> > > In file :            \drivers\char\synclink.c
> > > In function :    line_info(char *buf, struct mgsl_struct *info)
> > >
> > > The buffer *stat_buf* could be defined smaller.
> >
> >
> > Where's the patch?
>
> Never mind that.  Is this a patch we even want?  It's quite common to
> overallocate these small string buffers so that when someone makes a
> change and forgets to update the length, they don't overrun the buffer.

Valid point.  And the savings by reducing it aren't significant enough
to justify the potential error you mention.

> And what's with the backward slashes anyway?

Good question...maybe Christophe is using Winblows to report Linux errors?

Thanks,
Nish

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] drivers\char\synclink.c

[walter harms]

Nish Aravamudan wrote:
> On 11/25/05, Matthew Wilcox <matthew@wil.cx> wrote:
>> On Fri, Nov 25, 2005 at 01:34:03PM -0800, Nish Aravamudan wrote:
>>> On 11/25/05, Christophe Jaillet <christophe.jaillet@wanadoo.fr> wrote:
>>>> In file :            \drivers\char\synclink.c
>>>> In function :    line_info(char *buf, struct mgsl_struct *info)
>>>>
>>>> The buffer *stat_buf* could be defined smaller.
>>>
>>> Where's the patch?
>> Never mind that.  Is this a patch we even want?  It's quite common to
>> overallocate these small string buffers so that when someone makes a
>> change and forgets to update the length, they don't overrun the buffer.
> 
> Valid point.  And the savings by reducing it aren't significant enough
> to justify the potential error you mention.
> 
>> And what's with the backward slashes anyway?
> 

The problem with fixed size buffers is exactly THE problem if unwary 
change the length they cause errors.
ppl writing into buffer should use snprintf() (and friends).
To find the lenght any can use snprintf(NULL,...).
that cost time but fixes any possible overflow.

re,
   walter
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] drivers\char\synclink.c

[Nish Aravamudan]

On 11/26/05, walter harms <wharms@bfs.de> wrote:
>
>
> Nish Aravamudan wrote:
> > On 11/25/05, Matthew Wilcox <matthew@wil.cx> wrote:
> >> On Fri, Nov 25, 2005 at 01:34:03PM -0800, Nish Aravamudan wrote:
> >>> On 11/25/05, Christophe Jaillet <christophe.jaillet@wanadoo.fr> wrote:
> >>>> In file :            \drivers\char\synclink.c
> >>>> In function :    line_info(char *buf, struct mgsl_struct *info)
> >>>>
> >>>> The buffer *stat_buf* could be defined smaller.
> >>>
> >>> Where's the patch?
> >> Never mind that.  Is this a patch we even want?  It's quite common to
> >> overallocate these small string buffers so that when someone makes a
> >> change and forgets to update the length, they don't overrun the buffer.
> >
> > Valid point.  And the savings by reducing it aren't significant enough
> > to justify the potential error you mention.
> >
> >> And what's with the backward slashes anyway?
> >
>
> The problem with fixed size buffers is exactly THE problem if unwary
> change the length they cause errors.
> ppl writing into buffer should use snprintf() (and friends).
> To find the lenght any can use snprintf(NULL,...).
> that cost time but fixes any possible overflow.

I believe Matthew is referring to changing the code and not updating
the length? Not at run-time, but at compile-time, that is. CMIIW

Thanks,
Nish

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors