Skip to contentSkip to conversation

drivers\char\synclink.c

KJ Christophe Jaillet 4 participants 6 messages Nov 25, 2005 – Nov 26, 2005 Kernel janitors

lore
Share & tools
lore

Christophe Jaillet

author
#0 lore raw
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

_______________________________________________
Kernel-janitors mailing list
[email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors

#2 lore raw
In reply to Christophe Jaillet
Quoted 9 lines · Nish Aravamudan
Quoted 5 lines · Christophe Jaillet
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?

_______________________________________________
Kernel-janitors mailing list
[email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors

#3 lore raw
In reply to Christophe Jaillet
Quoted 6 lines · Matthew Wilcox
Quoted 5 lines · Nish Aravamudan
Quoted 4 lines · Christophe Jaillet
In file :            \drivers\char\synclink.c
In function :    line_info(char *buf, struct mgsl_struct *info)
Quoted 8 lines
Quoted 4 lines
Quoted 1 line

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.

Quoted 1 line

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
[email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors

#4 lore raw
In reply to Christophe Jaillet
Quoted 8 lines · Nish Aravamudan
Quoted 7 lines · Matthew Wilcox
Quoted 6 lines · Nish Aravamudan
Quoted 4 lines · Christophe Jaillet
In file :            \drivers\char\synclink.c
In function :    line_info(char *buf, struct mgsl_struct *info)

The buffer *stat_buf* could be defined smaller.

Quoted 10 lines
Quoted 4 lines
Quoted 1 line

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.

Quoted 1 line

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
[email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors

#5 lore raw
In reply to Christophe Jaillet
Quoted 19 lines · walter harms
Quoted 16 lines · Nish Aravamudan
Quoted 11 lines · Matthew Wilcox
Quoted 7 lines · Nish Aravamudan
Quoted 4 lines · Christophe Jaillet
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.

Quoted 8 lines
Quoted 2 lines
Quoted 1 line

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
[email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors