librelist archives

« back to archive

generate.c mmap usage?

generate.c mmap usage?

From:
Tom Enterline
Date:
2014-12-04 @ 19:50
Hi All,

The mmap version of load_text in generate.c continues to bother me. Code
follows:

Re: generate.c mmap usage?

From:
Tom Enterline
Date:
2014-12-04 @ 19:55
Sorry, sent too early... code is:
static uchar *
load_text(editbuffer_t *eb, const cvs_text *text)
    ...
    if ((fd = open(text->filename, O_RDONLY)) == -1)
    ....
    size = st.st_size;
    base = mmap(NULL, size, PROT_READ, MAP_SHARED, fd, 0);
    if (base == MAP_FAILED)
        fatal_system_error("mmap: %s %zu", text->filename, size);
    close(fd);

    munmap(eb->text_map.base, eb->text_map.size);
    eb->text_map.filename = text->filename;
    eb->text_map.base = base;
    eb->text_map.size = size;

    return base + offset;
}

static void
unload_all_text(editbuffer_t *eb)
{
    if (eb->text_map.filename) {
munmap(eb->text_map.base, eb->text_map.size);
eb->text_map.filename = NULL;
    }
}

so the filename is mmap'd, and immediately munmap'd,
but the base is used after the munmap. That looks like a
dangling pointer, is that correct usage?

To confuse the issue, the unload_all_text also does munmap.

On Thu, Dec 4, 2014 at 2:50 PM, Tom Enterline <tenterline@gmail.com> wrote:

> Hi All,
>
> The mmap version of load_text in generate.c continues to bother me. Code
> follows:
>
>

Re: generate.c mmap usage?

From:
Tom Enterline
Date:
2014-12-04 @ 19:58
I think I just answered my question.
The code is munmap'ing the previous mmap, not the new one.
So there is always one mmap in effect.

On Thu, Dec 4, 2014 at 2:55 PM, Tom Enterline <tenterline@gmail.com> wrote:

> Sorry, sent too early... code is:
> static uchar *
> load_text(editbuffer_t *eb, const cvs_text *text)
>     ...
>     if ((fd = open(text->filename, O_RDONLY)) == -1)
>     ....
>     size = st.st_size;
>     base = mmap(NULL, size, PROT_READ, MAP_SHARED, fd, 0);
>     if (base == MAP_FAILED)
>         fatal_system_error("mmap: %s %zu", text->filename, size);
>     close(fd);
>
>     munmap(eb->text_map.base, eb->text_map.size);
>     eb->text_map.filename = text->filename;
>     eb->text_map.base = base;
>     eb->text_map.size = size;
>
>     return base + offset;
> }
>
> static void
> unload_all_text(editbuffer_t *eb)
> {
>     if (eb->text_map.filename) {
> munmap(eb->text_map.base, eb->text_map.size);
> eb->text_map.filename = NULL;
>     }
> }
>
> so the filename is mmap'd, and immediately munmap'd,
> but the base is used after the munmap. That looks like a
> dangling pointer, is that correct usage?
>
> To confuse the issue, the unload_all_text also does munmap.
>
> On Thu, Dec 4, 2014 at 2:50 PM, Tom Enterline <tenterline@gmail.com>
> wrote:
>
>> Hi All,
>>
>> The mmap version of load_text in generate.c continues to bother me. Code
>> follows:
>>
>>
>
>