Scott Kostyshak <skost...@lyx.org> writes:

| On Thu, Sep 27, 2012 at 3:53 PM, Scott Kostyshak <skost...@lyx.org> wrote:
>> On Thu, Sep 27, 2012 at 1:51 PM, Julien Rioux
>> <jri...@physics.utoronto.ca> wrote:
>>> On 27/09/2012 7:17 AM, Scott Kostyshak wrote:
>>>>
>>>> -               ::write(pipefd, cmd.c_str(), cmd.length());
>>>> +                if (::write(pipefd, cmd.c_str(), cmd.length()) < 0)
>>>> +                        LYXERR0("Cannot write to pipe!");
>>>
>>>
>>> You're space-indenting in a tab-indented file here.
>>>
>>
>> Good catch. I thought I had git set up to detect these white space
>> errors but I guess not.
>>
>> Attached is the updated patch.
>>
>
| Attached are the three patches for branch (they are the same as in the
| last two emails but I wanted to put them all in one place).
>
| Should they go in?

Not my call, so I just look at the patches by themselves.

| From 877398ce2faa5751a62639c9c20978eb3ff33c35 Mon Sep 17 00:00:00 2001
| From: Scott Kostyshak <skost...@lyx.org>
| Date: Sun, 23 Sep 2012 04:58:19 -0400
| Subject: [PATCH 1/3] Remove unused function find_matching_brace
>
| find_matching_brace came over from a backport and is not being used.

This looks like a harmless change.

| From 35dcd7f6fb88a47bda9342e53d528cb521f34505 Mon Sep 17 00:00:00 2001
| From: Scott Kostyshak <skost...@lyx.org>
| Date: Thu, 27 Sep 2012 06:29:27 -0400
| Subject: [PATCH] Use a return value to report an unsuccessful write
>
| Use a return value in LyXComm::loadFilesInOtherInstance to give an
| error for a failed write to pipe.

Not sure.  Is the error handled any differently from the non-error path?
(except the message)?

We absolutely should check the return from ::write, but I am not sure if
it is enought to do _just_ that.

| From b7582fc12ac0b8130ff26f7a735b12e200e5a714 Mon Sep 17 00:00:00 2001
| From: Scott Kostyshak <skost...@lyx.org>
| Date: Thu, 27 Sep 2012 06:26:46 -0400
| Subject: [PATCH 2/3] Get rid of unused variable to eliminate warning
>
| The variable 'c' in Lexer::Pimpl::setFile was not being used.

This seem obvious, and it is already done on trunk in ea50cd71
(Which should/could be mentioned in the commit message)

-- 
   Lgb

Reply via email to