[PATCH] D25107: Expose PDBFileBuilder::finalizeMsfLayout.

[Rui Ueyama via llvm-commits]

On Fri, Sep 30, 2016 at 10:32 AM, Zachary Turner <zturner at google.com> wrote:

> zturner added inline comments.
>
>
> > PDBFileBuilder.h:53
> >
> > -private:
> > -  Expected<msf::MSFLayout> finalizeMsfLayout() const;
> > +  Error finalizeMsfLayout();
> >
>
> I'm not sure if making this public is a good idea.  This leaves open the
> possibility that someone could mis-use the class.
>
> I can think of two solutions that would still keep this private:
>
> 1. Changing `commit` so that instead of taking an `msf::WritableStream` it
> takes a filename.  Then you create the stream in there.
>

At first I thought that this approach makes things too tightly coupled, but
on second, it seems a practical approach. PDBFileBuilder builds a file, so
at end we naturally want to write it to a file. I'll update this patch with
this approach.

2. Change the implementation of `FileBufferByteStream` so that instead of
> taking a `FileOutputBuffer` to its constructor, it only takes a filename.
> Add a method to that class called `create(uint64_t size)`.  Then the
> signature of `commit` is changed to take an `msf::FileBufferByteStream` and
> after internally finalizing the layout, it calls
> `stream.create(layout.size)`.
>
> What do you think?
>
> https://reviews.llvm.org/D25107
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160930/b7560e91/attachment.html>