Code review comment for lp:~compiz-team/compiz/compiz.fix_1170013

Revision history for this message
Sam Spilsbury (smspillaz) wrote :

> Here a whitespace is missing:
>
> 172 + /* Redirect backed up fd to old fd location*/
>
> 274 + * as the child is using them at return*/
>

Fixed,

> You could remove the brackets here:
>
> 267 + else if (child == -1)
> 268 + {
> 269 + throw std::runtime_error (strerror (errno));
> 270 + }
>
> 306 + {
> 307 + /* waitpid () failed */
> 308 + throw std::runtime_error (strerror (errno));
> 309 + }
>
> 395 + else
> 396 + {
> 397 + /* There's nothing on the pipe, assume EOF */
> 398 + count = 0;
> 399 + }
>
> 445 + else
> 446 + {
> 447 + /* Extra space here to align with gtest output */
> 448 + std::cout << "[AUTOPILOT ] Pass test " << GetParam () <<
> std::endl;
> 449 + }

I've decided to remove the brackets only on the first. The inline comment necessitates brackets for readability purposes on the others.
>
> Typo:
>
> 389 + /* Always nul-terminate */

Fixed.

>
> Other than that LGTM AFAICT. ;)

« Back to merge proposal