[rtems-tools commit] rld-process: Add named tempfile constructor

Chris Johns chrisj at rtems.org
Wed Mar 31 22:20:47 UTC 2021


On 1/4/21 2:26 am, Alex White wrote:
> On Tue, Mar 30, 2021 at 10:30 PM Chris Johns <chrisj at rtems.org> wrote:
>>
>> Hi,
>>
>> When I looked at this on vc@ I saw it is broken. I am sorry I did not pick this
>> up in the review.
>>
>> On 31/3/21 5:20 am, Joel Sherrill wrote:
>>> Module:    rtems-tools
>>> Branch:    master
>>> Commit:    420d7a13672991a1480d06ac02190f2976b9253b
>>> Changeset: http://git.rtems.org/rtems-tools/commit/?id=420d7a13672991a1480d06ac02190f2976b9253b
>>>
>>> Author:    Alex White <alex.white at oarcorp.com>
>>> Date:      Wed Mar  3 09:48:00 2021 -0600
>>>
>>> rld-process: Add named tempfile constructor
>>>
>>> This adds a new tempfile constructor for creating a named tempfile
>>> rather than generating the name.
>>>
>>> ---
>>>
>>>  rtemstoolkit/rld-process.cpp | 11 +++++++++++
>>>  rtemstoolkit/rld-process.h   |  7 +++++++
>>>  2 files changed, 18 insertions(+)
>>>
>>> diff --git a/rtemstoolkit/rld-process.cpp b/rtemstoolkit/rld-process.cpp
>>> index 30e0605..4160759 100644
>>> --- a/rtemstoolkit/rld-process.cpp
>>> +++ b/rtemstoolkit/rld-process.cpp
>>> @@ -169,6 +169,17 @@ namespace rld
>>>        _name = temporaries.get (suffix, _keep);
>>>      }
>>>
>>> +    tempfile::tempfile (const std::string& name,
>>> +                        const std::string& suffix,
>>> +                        bool               _keep)
>>
>> As a side issue this should be `keep` and not `_keep`. Could this please be
>> fixed with fix that is needed.
>>
>>> +      : _name(name + suffix),
>>> +        suffix(suffix),
>>> +        overridden (false),
>>> +        fd (-1),
>>> +        level (0)
>>> +    {
>>> +    }
>>
>> This constructor is empty and so the file is not created in the temp path on
>> Unix or Windows and it is not registered to be deleted. I am sorry I did not
>> notice this before. How was this change tested?
>>
>> Now I think about this change I am not sure it is right. Why you are using this
>> interface when the file is not in a temp directory and is not being deleted?
> 
> Hi Chris,
> 
> This patch set up the interfaces used in the "covoar: Add option to create named objdumps" patch that I sent.
> 
> We determined that patch was not needed given its hacked-togetherness along with future speedups that we had in mind which would make it obsolete anyway.
> 
> This patch should have been removed from the set to be committed, but I forgot to send out a notice to exclude it. Sorry for the confusion.
> 
> This commit should be reverted.

Ah OK. Could you or Joel please close #4368 with this as a comment?

Thanks
Chris


More information about the devel mailing list