Login | Register For Free | Help
Search for: (Advanced)

Mailing List Archive: Linux: Kernel

[PATCH] Extend core dump note section to contain file names of mapped files

 

 

Linux kernel RSS feed   Index | Next | Previous | View Threaded


vda.linux at googlemail

Mar 31, 2012, 1:51 PM

Post #1 of 11 (134 views)
Permalink
[PATCH] Extend core dump note section to contain file names of mapped files

Hi,

[forgot to CC lkml, resending]

While working with core dump analysis, it struck me how much
PITA is caused merely by the fact that names of loaded binary
and libraries are not known.

gdb retrieves loaded library names by examining dynamic loader's
data stored in the core dump's data segments. It uses intimate
knowledge how and where dynamic loader keeps the list of loaded
libraries. (Meaning that it will break if non-standard loader
is used).

And, as Jan explained to me, it depends on knowing where
the linked list of libraries starts, which requires knowing binary
which was running. IIRC there is no easy and reasonably foolproof
way to determine binary's name. (Looking at argv[0] on stack
is not reasonably foolproof).

Which is *ridiculous*. We *know* the list of mapped files
at core dump generation time.

I propose to save this information in core dump, as a new note
in note segment.

This note has the following format:

long count // how many files are mapped
long page_size // units for file_ofs
array of [COUNT] elements of
long start
long end
long file_ofs
followed by COUNT filenames in ASCII: "FILE1" NUL "FILE2" NUL...

The attached patch implements this.

Since list of mapped files can be large (/proc/`pidof firefox`/maps
on my machine right now is 38k), I allocate the space for note
via vmalloc, and also have a sanity limit of 4 megabytes.
(Maybe we should make it smaller?)
Oleg suggested using a linked list of smaller structures instead of
using a potentially large contiguous block, and I tried it,
but resulting code was significantly more ugly (for my taste).

The patch is run-tested.

For testing, I sent ABRT signal to a running /usr/bin/md5sum.

"readelf -aW core" shows the new note as:

Notes at offset 0x00000274 with length 0x00000990:
Owner Data size Description
CORE 0x00000090 NT_PRSTATUS (prstatus structure)
CORE 0x0000007c NT_PRPSINFO (prpsinfo structure)
CORE 0x000000a0 NT_AUXV (auxiliary vector)
CORE 0x00000168 Unknown note type: (0x46494c45)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^new note^^^^^^^^^^

In hex format:
05 00 00 00 |................|
00000460 68 01 00 00 45 4c 49 46 46 49 4c 45 00 00 00 00 |h...ELIFCORE....|
00000470 0b 00 00 00 00 10 00 00 00 80 17 00 00 f0 31 00 |..............1.|
00000480 00 00 00 00 00 f0 31 00 00 00 32 00 a7 01 00 00 |......1...2.....|
00000490 00 00 32 00 00 20 32 00 a7 01 00 00 00 20 32 00 |..2.. 2...... 2.|
000004a0 00 30 32 00 a9 01 00 00 00 50 69 00 00 60 6b 00 |.02......Pi..`k.|
000004b0 00 00 00 00 00 60 6b 00 00 70 6b 00 20 00 00 00 |.....`k..pk. ...|
000004c0 00 70 6b 00 00 80 6b 00 21 00 00 00 00 80 04 08 |.pk...k.!.......|
000004d0 00 00 05 08 00 00 00 00 00 00 05 08 00 10 05 08 |................|
000004e0 07 00 00 00 00 10 05 08 00 20 05 08 08 00 00 00 |......... ......|
000004f0 00 20 52 b7 00 20 72 b7 00 00 00 00 2f 6c 69 62 |. R.. r...../lib|
00000500 2f 6c 69 62 63 2d 32 2e 31 34 2e 39 30 2e 73 6f |/libc-2.14.90.so|
00000510 00 2f 6c 69 62 2f 6c 69 62 63 2d 32 2e 31 34 2e |./lib/libc-2.14.|
00000520 39 30 2e 73 6f 00 2f 6c 69 62 2f 6c 69 62 63 2d |90.so./lib/libc-|
00000530 32 2e 31 34 2e 39 30 2e 73 6f 00 2f 6c 69 62 2f |2.14.90.so./lib/|
00000540 6c 69 62 63 2d 32 2e 31 34 2e 39 30 2e 73 6f 00 |libc-2.14.90.so.|
00000550 2f 6c 69 62 2f 6c 64 2d 32 2e 31 34 2e 39 30 2e |/lib/ld-2.14.90.|
00000560 73 6f 00 2f 6c 69 62 2f 6c 64 2d 32 2e 31 34 2e |so./lib/ld-2.14.|
00000570 39 30 2e 73 6f 00 2f 6c 69 62 2f 6c 64 2d 32 2e |90.so./lib/ld-2.|
00000580 31 34 2e 39 30 2e 73 6f 00 2f 75 73 72 2f 62 69 |14.90.so./usr/bi|
00000590 6e 2f 6d 64 35 73 75 6d 00 2f 75 73 72 2f 62 69 |n/md5sum./usr/bi|
000005a0 6e 2f 6d 64 35 73 75 6d 00 2f 75 73 72 2f 62 69 |n/md5sum./usr/bi|
000005b0 6e 2f 6d 64 35 73 75 6d 00 2f 75 73 72 2f 6c 69 |n/md5sum./usr/li|
000005c0 62 2f 6c 6f 63 61 6c 65 2f 6c 6f 63 61 6c 65 2d |b/locale/locale-|
000005d0 61 72 63 68 69 76 65 00 |archive.

--
vda
Attachments: file_note.patch (3.32 KB)


andi at firstfloor

Mar 31, 2012, 8:13 PM

Post #2 of 11 (124 views)
Permalink
Re: [PATCH] Extend core dump note section to contain file names of mapped files [In reply to]

> I propose to save this information in core dump, as a new note
> in note segment.

Seems like a good idea but rather than write complicated code i would just reuse
the /proc/*/maps code and dump it in that format?

-Andi

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


hpa at zytor

Mar 31, 2012, 8:20 PM

Post #3 of 11 (126 views)
Permalink
Re: [PATCH] Extend core dump note section to contain file names of mapped files [In reply to]

On 03/31/2012 08:13 PM, Andi Kleen wrote:
>> I propose to save this information in core dump, as a new note
>> in note segment.
>
> Seems like a good idea but rather than write complicated code i would just reuse
> the /proc/*/maps code and dump it in that format?
>

Does /proc/*/maps handle oddball characters in filenames?

-hpa


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


andi at firstfloor

Apr 1, 2012, 2:44 AM

Post #4 of 11 (125 views)
Permalink
Re: [PATCH] Extend core dump note section to contain file names of mapped files [In reply to]

On Sat, Mar 31, 2012 at 08:20:18PM -0700, H. Peter Anvin wrote:
> On 03/31/2012 08:13 PM, Andi Kleen wrote:
> >> I propose to save this information in core dump, as a new note
> >> in note segment.
> >
> > Seems like a good idea but rather than write complicated code i would just reuse
> > the /proc/*/maps code and dump it in that format?
> >
>
> Does /proc/*/maps handle oddball characters in filenames?

It doesn't handle new lines well, but then if someone does that they
break all programs that parse maps for other reasons (and likely
a lot of other things too) anyways.

Other than that it should be ok: the file name is just the last component
on the line.

-Andi
--
ak [at] linux -- Speaking for myself only.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


vda.linux at googlemail

Apr 1, 2012, 6:33 AM

Post #5 of 11 (120 views)
Permalink
Re: [PATCH] Extend core dump note section to contain file names of mapped files [In reply to]

On Sunday 01 April 2012 05:13, Andi Kleen wrote:
> > I propose to save this information in core dump, as a new note
> > in note segment.
>
> Seems like a good idea but rather than write complicated code i would just reuse
> the /proc/*/maps code and dump it in that format?

I looked at this code (it's in fs/proc/task_mmu.c).

It uses seq_printf(). It will need refactoring if we would
want to use it for purposes other than /proc pseudo-file
generation.

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


andi at firstfloor

Apr 1, 2012, 9:53 AM

Post #6 of 11 (125 views)
Permalink
Re: [PATCH] Extend core dump note section to contain file names of mapped files [In reply to]

> It uses seq_printf(). It will need refactoring if we would
> want to use it for purposes other than /proc pseudo-file
> generation.

seq_file just writes to a buffer. I'm sure there's a way to get at that
buffer. In theory you could also just copy using read/write and a small temp
buffer. The only problem would be to stop it taking the vm sem.

-Andi
--
ak [at] linux -- Speaking for myself only.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


oleg at redhat

Apr 1, 2012, 5:24 PM

Post #7 of 11 (123 views)
Permalink
Re: [PATCH] Extend core dump note section to contain file names of mapped files [In reply to]

On 04/01, Andi Kleen wrote:
>
> > I propose to save this information in core dump, as a new note
> > in note segment.
>
> Seems like a good idea but rather than write complicated code

I agree, I feel it can be simplified...

> i would just reuse
> the /proc/*/maps code and dump it in that format?

I must have missed something. Do you really suggest to use
show_pid_map/etc?

If nothing else, this code depends on CONFIG_PROC_FS. But in any
case I think this will only complicate fill_files_note().

coredump is "simple", we are the last thread which can play with
this ->mm. We do not need locks, we do not need the "restart after
we dropped mmap_sem" logic. We know that the task_struct can't go
away. The only problem is rename, that is why we can't allocate the
whole buffer beforehand.

OK, I must have missed something ;)

Oleg.

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


palves at redhat

Apr 2, 2012, 4:20 AM

Post #8 of 11 (117 views)
Permalink
Re: [PATCH] Extend core dump note section to contain file names of mapped files [In reply to]

On 04/02/2012 01:24 AM, Oleg Nesterov wrote:

> On 04/01, Andi Kleen wrote:
>> >
>> >
>> > Seems like a good idea but rather than write complicated code
> I agree, I feel it can be simplified...
>
>> > i would just reuse
>> > the /proc/*/maps code and dump it in that format?
> I must have missed something. Do you really suggest to use
> show_pid_map/etc?
>
> If nothing else, this code depends on CONFIG_PROC_FS. But in any
> case I think this will only complicate fill_files_note().
>
> coredump is "simple", we are the last thread which can play with
> this ->mm. We do not need locks, we do not need the "restart after
> we dropped mmap_sem" logic. We know that the task_struct can't go
> away. The only problem is rename, that is why we can't allocate the
> whole buffer beforehand.
>
> OK, I must have missed something ;)


In my perspective, it's about having a single format to consume.
Having only one format to care for in either the core or when
debugging a live process simplifies things for all parties involved.

(And it doesn't seem a good idea to me to have a better format that
handles more things when debugging cores than what tools have available
when debugging a live process. If /proc/*/maps doesn't handle something
we might care for, then fix that too, not just the core note.)

The kernel. The man pages. Userspace tools. E.g., GDB can trivially
be made to hook a new core note with /proc/*/maps-like contents with
the "(gdb) info proc maps" command (an often requested feature).

I even wish that more (if not all) of /proc/ 's objects were
copied verbatim (as much as possible) to a core dump.

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


oleg at redhat

Jul 11, 2012, 8:15 AM

Post #9 of 11 (80 views)
Permalink
Re: [PATCH] Extend core dump note section to contain file names of mapped files [In reply to]

On 07/11, Denys Vlasenko wrote:
>
> I propose to save this information in core dump, as a new note
> in note segment.

Denys, I am in no position to discuss whether we need this change or not,
format, etc. I'll only try to comment the code.

And please do not use the attachments ;)

> +static void fill_files_note(struct memelfnote *note)
> +{
> + struct vm_area_struct *vma;
> + struct file *file;
> + unsigned count, word_count, size, remaining;
> + long *data;
> + long *start_end_ofs;
> + char *name;
> +
> + count = 0;
> + for (vma = current->mm->mmap; vma != NULL; vma = vma->vm_next) {
> + file = vma->vm_file;
> + if (!file)
> + continue;
> + count++;
> + if (count >= MAX_FILE_NOTE_SIZE / 64) /* paranoia check */
> + goto err;

Why this check? If count is huge, then...

> + size = count * 64;
> + word_count = 2 + 3 * count;
> + alloc:
> + if (size >= MAX_FILE_NOTE_SIZE) /* paranoia check */
> + goto err;

we should detect this case before the first alloc?

> + size = (size + PAGE_SIZE - 1) & (-PAGE_SIZE);

Well, I'd suggest PAGE_MASK instead of -PAGE_SIZE. Better yet,

size = round_up(size, PAGE_SIZE);

> + if (remaining == 0) {
> + try_new_size:
> + vfree(data);
> + size = size * 5 / 4;
> + goto alloc;
> + }
> + filename = d_path(&file->f_path, name, remaining);
> + if (IS_ERR(filename)) {
> + if (PTR_ERR(filename) == -ENAMETOOLONG)
> + goto try_new_size;

This looks like unnecessary complication to me, or I missed something.
d_path(..., buflen) should handle the "buflen == 0" case correctly, so
afacics you can remove the "if (remaining == 0)" block and move this
free-and-goto-alloc code under the -ENAMETOOLONG check.

> + while ((remaining--, *name++ = *filename++) != '\0')
> + continue;

Well, perhaps this is just me... but this looks a bit too complex
to me ;) I won't insist, but

do
remaining--;
while ((*name++ = *filename++));

looks more understandable, imho.

Or even

/* d_path() fills the end of the buffer */
remaining = name - filename;
strcpy(name, filename);

Oleg.

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


jmfoote at cert

Jul 11, 2012, 8:40 AM

Post #10 of 11 (82 views)
Permalink
RE: [PATCH] Extend core dump note section to contain file names of mapped files [In reply to]

Hello all,

For what it's worth several security teams that use the CERT Triage Tools have requested a feature (post-mortem analysis of core files) that will be supported by this functionality. This feature in the CERT Triage Tools would be more complete if siginfo was included in the core files as well, but this is a good start.

Jon

-----Original Message-----
From: Denys Vlasenko [mailto:vda.linux [at] gmail]
Sent: Wednesday, July 11, 2012 6:36 AM
To: linux-kernel [at] vger; Jonathan M. Foote; H. J. Lu; Ingo Molnar; H. Peter Anvin; Andi Kleen
Cc: Oleg Nesterov; Denys Vlasenko; Jan Kratochvil
Subject: [PATCH] Extend core dump note section to contain file names of mapped files

Hi,

Resending the patch after a while.
Jonathan, developer of CERT Triage Tools, expressed the need to have this information, CCing him.

But before looking at the attached patch, we need a ruling.

In the last review it was proposed to maybe generate this information in the form of ASCII text, a-la /proc/PID/maps.

This actually is a good idea, but regretfully, it come a few decades too late, the rest of core file auxiliary information is traditionally encoded in binary structures.

Please, can someone with authority in this area decide whether we want to be unorthodox and use ASCII encoding for the whole thing, or not?

If the decision will be to use ASCII, I will need to rework the patch.

Otherwise, please take a look at attached patch which implements creation of a new note in binary format and let me know what do you think of it.

Original patch and description follows

* * * * * * * * * * * * * * * * * * * *

While working with core dump analysis, it struck me how much PITA is caused merely by the fact that names of loaded binary and libraries are not known.

gdb retrieves loaded library names by examining dynamic loader's data stored in the core dump's data segments. It uses intimate knowledge how and where dynamic loader keeps the list of loaded libraries. (Meaning that it will break if non-standard loader is used).

And, as Jan explained to me, it depends on knowing where the linked list of libraries starts, which requires knowing binary which was running. IIRC there is no easy and reasonably foolproof way to determine binary's name. (Looking at argv[0] on stack is not reasonably foolproof).

Which is *ridiculous*. We *know* the list of mapped files at core dump generation time.

I propose to save this information in core dump, as a new note in note segment.

This note has the following format:

long count // how many files are mapped
long page_size // units for file_ofs
array of [COUNT] elements of
long start
long end
long file_ofs
followed by COUNT filenames in ASCII: "FILE1" NUL "FILE2" NUL...
The attached patch implements this.

Since list of mapped files can be large (/proc/`pidof firefox`/maps on my machine right now is 38k), I allocate the space for note via vmalloc, and also have a sanity limit of 4 megabytes.
(Maybe we should make it smaller?)
Oleg suggested using a linked list of smaller structures instead of using a potentially large contiguous block, and I tried it, but resulting code was significantly more ugly (for my taste).

The patch is run-tested.

For testing, I sent ABRT signal to a running /usr/bin/md5sum.

"readelf -aW core" shows the new note as:

Notes at offset 0x00000274 with length 0x00000990:
Owner Data size Description
CORE 0x00000090 NT_PRSTATUS (prstatus structure)
CORE 0x0000007c NT_PRPSINFO (prpsinfo structure)
CORE 0x000000a0 NT_AUXV (auxiliary vector)
CORE 0x00000168 Unknown note type: (0x46494c45)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^new note^^^^^^^^^^ In hex format:
05 00 00 00 |................|
00000460 68 01 00 00 45 4c 49 46 46 49 4c 45 00 00 00 00 |h...ELIFCORE....|
00000470 0b 00 00 00 00 10 00 00 00 80 17 00 00 f0 31 00 |..............1.|
00000480 00 00 00 00 00 f0 31 00 00 00 32 00 a7 01 00 00 |......1...2.....|
00000490 00 00 32 00 00 20 32 00 a7 01 00 00 00 20 32 00 |..2.. 2...... 2.|
000004a0 00 30 32 00 a9 01 00 00 00 50 69 00 00 60 6b 00 |.02......Pi..`k.|
000004b0 00 00 00 00 00 60 6b 00 00 70 6b 00 20 00 00 00 |.....`k..pk. ...|
000004c0 00 70 6b 00 00 80 6b 00 21 00 00 00 00 80 04 08 |.pk...k.!.......|
000004d0 00 00 05 08 00 00 00 00 00 00 05 08 00 10 05 08 |................|
000004e0 07 00 00 00 00 10 05 08 00 20 05 08 08 00 00 00 |......... ......|
000004f0 00 20 52 b7 00 20 72 b7 00 00 00 00 2f 6c 69 62 |. R.. r...../lib|
00000500 2f 6c 69 62 63 2d 32 2e 31 34 2e 39 30 2e 73 6f |/libc-2.14.90.so|
00000510 00 2f 6c 69 62 2f 6c 69 62 63 2d 32 2e 31 34 2e |./lib/libc-2.14.|
00000520 39 30 2e 73 6f 00 2f 6c 69 62 2f 6c 69 62 63 2d |90.so./lib/libc-|
00000530 32 2e 31 34 2e 39 30 2e 73 6f 00 2f 6c 69 62 2f |2.14.90.so./lib/|
00000540 6c 69 62 63 2d 32 2e 31 34 2e 39 30 2e 73 6f 00 |libc-2.14.90.so.|
00000550 2f 6c 69 62 2f 6c 64 2d 32 2e 31 34 2e 39 30 2e |/lib/ld-2.14.90.|
00000560 73 6f 00 2f 6c 69 62 2f 6c 64 2d 32 2e 31 34 2e |so./lib/ld-2.14.|
00000570 39 30 2e 73 6f 00 2f 6c 69 62 2f 6c 64 2d 32 2e |90.so./lib/ld-2.|
00000580 31 34 2e 39 30 2e 73 6f 00 2f 75 73 72 2f 62 69 |14.90.so./usr/bi|
00000590 6e 2f 6d 64 35 73 75 6d 00 2f 75 73 72 2f 62 69 |n/md5sum./usr/bi|
000005a0 6e 2f 6d 64 35 73 75 6d 00 2f 75 73 72 2f 62 69 |n/md5sum./usr/bi|
000005b0 6e 2f 6d 64 35 73 75 6d 00 2f 75 73 72 2f 6c 69 |n/md5sum./usr/li|
000005c0 62 2f 6c 6f 63 61 6c 65 2f 6c 6f 63 61 6c 65 2d |b/locale/locale-|
000005d0 61 72 63 68 69 76 65 00 |archive.

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


vda.linux at gmail

Jul 12, 2012, 12:41 PM

Post #11 of 11 (81 views)
Permalink
Re: [PATCH] Extend core dump note section to contain file names of mapped files [In reply to]

On Wednesday 11 July 2012 17:15, Oleg Nesterov wrote:
> On 07/11, Denys Vlasenko wrote:
> >
> > I propose to save this information in core dump, as a new note
> > in note segment.
>
> Denys, I am in no position to discuss whether we need this change or not,
> format, etc. I'll only try to comment the code.
>
> And please do not use the attachments ;)
>
> > +static void fill_files_note(struct memelfnote *note)
> > +{
> > + struct vm_area_struct *vma;
> > + struct file *file;
> > + unsigned count, word_count, size, remaining;
> > + long *data;
> > + long *start_end_ofs;
> > + char *name;
> > +
> > + count = 0;
> > + for (vma = current->mm->mmap; vma != NULL; vma = vma->vm_next) {
> > + file = vma->vm_file;
> > + if (!file)
> > + continue;
> > + count++;
> > + if (count >= MAX_FILE_NOTE_SIZE / 64) /* paranoia check */
> > + goto err;
>
> Why this check? If count is huge, then...
>
> > + size = count * 64;
> > + word_count = 2 + 3 * count;
> > + alloc:
> > + if (size >= MAX_FILE_NOTE_SIZE) /* paranoia check */
> > + goto err;
>
> we should detect this case before the first alloc?

Unless count * 64 overflows an int :)
As I said in the comment: paranoia.

Perhaps that's TOO MUCH of paranoia. Removing.

> > + size = (size + PAGE_SIZE - 1) & (-PAGE_SIZE);
>
> Well, I'd suggest PAGE_MASK instead of -PAGE_SIZE. Better yet,
>
> size = round_up(size, PAGE_SIZE);
>
> > + if (remaining == 0) {
> > + try_new_size:
> > + vfree(data);
> > + size = size * 5 / 4;
> > + goto alloc;
> > + }
> > + filename = d_path(&file->f_path, name, remaining);
> > + if (IS_ERR(filename)) {
> > + if (PTR_ERR(filename) == -ENAMETOOLONG)
> > + goto try_new_size;
>
> This looks like unnecessary complication to me, or I missed something.
> d_path(..., buflen) should handle the "buflen == 0" case correctly, so
> afacics you can remove the "if (remaining == 0)" block and move this
> free-and-goto-alloc code under the -ENAMETOOLONG check.
>
> > + while ((remaining--, *name++ = *filename++) != '\0')
> > + continue;
>
> Well, perhaps this is just me... but this looks a bit too complex
> to me ;) I won't insist, but
>
> do
> remaining--;
> while ((*name++ = *filename++));
>
> looks more understandable, imho.

Okay.


> Or even
>
> /* d_path() fills the end of the buffer */
> remaining = name - filename;
> strcpy(name, filename);

This does not advance "name" pointer... oh...
it's actually clever! But it'll fail if we took

/* continue; -- WRONG, we must have COUNT elements */
filename = "";
}

branch just above... I will use an open-coded loop for now.

Sending v2 in a moment.

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

Linux kernel RSS feed   Index | Next | Previous | View Threaded
 
 


Interested in having your list archived? Contact Gossamer Threads
 
  Web Applications & Managed Hosting Powered by Gossamer Threads Inc.