http://d.puremagic.com/issues/show_bug.cgi?id=9421

           Summary: (Compiler internals) Change OutBuffer's interface
           Product: D
           Version: D1 & D2
          Platform: All
        OS/Version: All
            Status: NEW
          Severity: enhancement
          Priority: P2
         Component: DMD
        AssignedTo: nobody@puremagic.com
        ReportedBy: yebblies@gmail.com


--- Comment #0 from yebblies <yebblies@gmail.com> 2013-01-29 19:31:03 EST ---
Using OutBuffer makes it easy to forget to zero-terminate strings and create dangling pointers.  I'm think we should make OutBuffer a little less error prone.

Changes:
- Get rid of toChars
- Add extractString which checks for a '\0' then takes ownership of the
internal buffer
- Add scopedString which checks for a '\0' then return a reference to the
internal buffer
- Add scopedData which returns a reference to the internal buffer

Benefit:
- Nobody will use toChars thinking that it copies/owns the data
- Harder to forget to zero-terminate strings
- Hopefully less casting

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=9421


Andrej Mitrovic <andrej.mitrovich@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |andrej.mitrovich@gmail.com


--- Comment #1 from Andrej Mitrovic <andrej.mitrovich@gmail.com> 2013-02-06 18:22:42 PST ---
(In reply to comment #0)
> Using OutBuffer makes it easy to forget to zero-terminate strings and create dangling pointers.  I'm think we should make OutBuffer a little less error prone.
> 
> Changes:
> - Get rid of toChars

There's also toString, which seems to be almost the same as toChars. And the OutBuffer implementation is split across src/root/root.c and src/backend/outbuf.c

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=9421



--- Comment #2 from Andrej Mitrovic <andrej.mitrovich@gmail.com> 2013-02-06 18:29:02 PST ---
So if I got it right we'd have:

char *OutBuffer::extractString()
{
    writeByte(0);
    char *p;
    p = (char *)data;
    data = NULL;
    offset = 0;
    size = 0;
    return p;
}

char *OutBuffer::scopedString()
{
    writeByte(0);
    return (char *)data;
}

char *OutBuffer::scopedData()
{
    return (char *)data;
}

Is that right?

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=9421



--- Comment #3 from yebblies <yebblies@gmail.com> 2013-02-07 14:55:05 EST ---
(In reply to comment #2)

The functions shouldn't append a '\0' if data already ends with one, and I'm not sure scopedString should _ever_ modify it.  Other than that it looks right.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=9421



--- Comment #4 from Andrej Mitrovic <andrej.mitrovich@gmail.com> 2013-02-07 11:44:09 PST ---
(In reply to comment #3)
> (In reply to comment #2)
> 
> The functions shouldn't append a '\0' if data already ends with one, and I'm not sure scopedString should _ever_ modify it.  Other than that it looks right.

Then I don't understand what you mean by: "Add scopedString which checks for a '\0' then return a reference to the internal buffer".

It checks for a \0, and does what if it doesn't find it?

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=9421



--- Comment #5 from yebblies <yebblies@gmail.com> 2013-02-08 11:43:19 EST ---
(In reply to comment #4)
> (In reply to comment #3)
> > (In reply to comment #2)
> > 
> > The functions shouldn't append a '\0' if data already ends with one, and I'm not sure scopedString should _ever_ modify it.  Other than that it looks right.
> 
> Then I don't understand what you mean by: "Add scopedString which checks for a '\0' then return a reference to the internal buffer".
> 
> It checks for a \0, and does what if it doesn't find it?

assert(0)

The idea being that getting a pointer to the internal string shouldn't change the contents.  Maybe it would be more useful to append a zero, I don't know.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=9421



--- Comment #6 from Andrej Mitrovic <andrej.mitrovich@gmail.com> 2013-02-12 20:30:07 PST ---
(In reply to comment #5)
> The idea being that getting a pointer to the internal string shouldn't change the contents.

Ok, but then what do we need scopedData for? If we're returning a char* and it's not zero-terminated I don't see how it can be used.

So far I've got this:

/** Extract a zero-terminated string.
    The caller takes ownership. */
const char *OutBuffer::extractString()
{
    if (!data || data[offset] != '\0')
        writeByte(0);
    char *p;
    p = (char *)data;
    data = NULL;
    offset = 0;
    size = 0;
    return p;
}

/** Verify the internal buffer is zero-terminated
    and return a reference to it. */
const char *OutBuffer::scopedString()
{
    assert(data && data[offset] == '\0');
    return (char *)data;
}

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=9421



--- Comment #7 from Andrej Mitrovic <andrej.mitrovich@gmail.com> 2013-02-12 20:31:04 PST ---
(In reply to comment #6)
> (In reply to comment #5)
> > The idea being that getting a pointer to the internal string shouldn't change the contents.
> 
> Ok, but then what do we need scopedData for? If we're returning a char* and it's not zero-terminated I don't see how it can be used.
> 
> So far I've got this:

Those casts should have been (const char*).

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=9421



--- Comment #8 from Andrej Mitrovic <andrej.mitrovich@gmail.com> 2013-02-12 20:37:10 PST ---
(In reply to comment #7)
> (In reply to comment #6)
> > (In reply to comment #5)
> > > The idea being that getting a pointer to the internal string shouldn't change the contents.
> > 
> > Ok, but then what do we need scopedData for? If we're returning a char* and it's not zero-terminated I don't see how it can be used.
> > 
> > So far I've got this:
> 
> Those casts should have been (const char*).

Or maybe they should remain (char *) if ownership is taken. :)

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=9421



--- Comment #9 from yebblies <yebblies@gmail.com> 2013-02-13 15:39:57 EST ---
(In reply to comment #6)
> (In reply to comment #5)
> > The idea being that getting a pointer to the internal string shouldn't change the contents.
> 
> Ok, but then what do we need scopedData for? If we're returning a char* and it's not zero-terminated I don't see how it can be used.
> 

You would use it in the same places as scopedString, but when using OutBuffer for binary instead of text.  i.e. when you know the data won't be escaped.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------