kernelnewbies.kernelnewbies.org archive mirror
 help / color / mirror / Atom feed
* Possible error in debugfs/file.c
@ 2014-12-02 16:13 land.ho87 at gmail.com
  2014-12-02 16:30 ` Bjørn Mork
  2014-12-02 16:45 ` Sudip Mukherjee
  0 siblings, 2 replies; 3+ messages in thread
From: land.ho87 at gmail.com @ 2014-12-02 16:13 UTC (permalink / raw)
  To: kernelnewbies

I'm just reading the kernel source and came across this which doesn't look quite right to me:
616         size_t size = strlen(file->private_data);

strlen is used here when the pointer points to type:
567 struct array_data {
568         void *array;
569         u32 elements;
570 };

I think line 616 should probably be something like:
size_t size = file->private_data.elements*sizeof(u32);

I think strlen would terminate incorrectly on any null byte, and is unnecessary
since the data is already counted.

Is this a legitimate criticism?

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Possible error in debugfs/file.c
  2014-12-02 16:13 Possible error in debugfs/file.c land.ho87 at gmail.com
@ 2014-12-02 16:30 ` Bjørn Mork
  2014-12-02 16:45 ` Sudip Mukherjee
  1 sibling, 0 replies; 3+ messages in thread
From: Bjørn Mork @ 2014-12-02 16:30 UTC (permalink / raw)
  To: kernelnewbies

land.ho87 at gmail.com writes:

> I'm just reading the kernel source and came across this which doesn't look quite right to me:
> 616         size_t size = strlen(file->private_data);
>
> strlen is used here when the pointer points to type:
> 567 struct array_data {
> 568         void *array;
> 569         u32 elements;
> 570 };

No, it doesn't.  file->private_data points to a string buffer allocated
in u32_array_open() and filled with a string representation of the
struct array_data, using u32_format_array().

So calling strlen() on it is perfectly valid and reasonable.




Bj?rn

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Possible error in debugfs/file.c
  2014-12-02 16:13 Possible error in debugfs/file.c land.ho87 at gmail.com
  2014-12-02 16:30 ` Bjørn Mork
@ 2014-12-02 16:45 ` Sudip Mukherjee
  1 sibling, 0 replies; 3+ messages in thread
From: Sudip Mukherjee @ 2014-12-02 16:45 UTC (permalink / raw)
  To: kernelnewbies

Please also see line 596..
file->private_data is buf and that is a string terminated by NULL.

thanks
sudip
On Dec 2, 2014 9:45 PM, <land.ho87@gmail.com> wrote:

> I'm just reading the kernel source and came across this which doesn't look
> quite right to me:
> 616         size_t size = strlen(file->private_data);
>
> strlen is used here when the pointer points to type:
> 567 struct array_data {
> 568         void *array;
> 569         u32 elements;
> 570 };
>
> I think line 616 should probably be something like:
> size_t size = file->private_data.elements*sizeof(u32);
>
> I think strlen would terminate incorrectly on any null byte, and is
> unnecessary
> since the data is already counted.
>
> Is this a legitimate criticism?
>
> _______________________________________________
> Kernelnewbies mailing list
> Kernelnewbies at kernelnewbies.org
> http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20141202/cbffc910/attachment.html 

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2014-12-02 16:45 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-12-02 16:13 Possible error in debugfs/file.c land.ho87 at gmail.com
2014-12-02 16:30 ` Bjørn Mork
2014-12-02 16:45 ` Sudip Mukherjee

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).