code review 6017043: os: avoid panic when testing errors on Plan 9

os: avoid panic when testing errors on Plan 9

[ality, 2012-04-12 13:33:24]

Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://code.google.com/p/go/

[bradfitz, 2012-04-12 14:13:42]

LGTM

searching English for a substring seems fragile, but that was there before.
 this is an improvement.

On Thu, Apr 12, 2012 at 6:33 AM, <ality@pbrane.org> wrote:

> Reviewers: golang-dev_googlegroups.com,
>
> Message:
> Hello golang-dev@googlegroups.com,
>
> I'd like you to review this change to
> https://code.google.com/p/go/
>
>
> Description:
> os: avoid panic when testing errors on Plan 9
>
> Please review this at
http://codereview.appspot.com/**6017043/<http://codereview.appspot.com/6017043/>
>
> Affected files:
>  M src/pkg/os/error_plan9.go
>
>
> Index: src/pkg/os/error_plan9.go
> ==============================**==============================**=======
> --- a/src/pkg/os/error_plan9.go
> +++ b/src/pkg/os/error_plan9.go
> @@ -5,6 +5,9 @@
>  package os
>
>  func isExist(err error) bool {
> +       if err == nil {
> +               return false
> +       }
>        if pe, ok := err.(*PathError); ok {
>                err = pe.Err
>        }
> @@ -12,6 +15,9 @@
>  }
>
>  func isNotExist(err error) bool {
> +       if err == nil {
> +               return false
> +       }
>        if pe, ok := err.(*PathError); ok {
>                err = pe.Err
>        }
> @@ -19,6 +25,9 @@
>  }
>
>  func isPermission(err error) bool {
> +       if err == nil {
> +               return false
> +       }
>        if pe, ok := err.(*PathError); ok {
>                err = pe.Err
>        }
>
>
>

[akumar_mail.nanosouffle.net, 2012-04-12 20:22:08]

LGTM.

On 12 April 2012 07:13, Brad Fitzpatrick <bradfitz@golang.org> wrote:
> LGTM
>
> searching English for a substring seems fragile, but that was there before.
>  this is an improvement.
>
> On Thu, Apr 12, 2012 at 6:33 AM, <ality@pbrane.org> wrote:
>>
>> Reviewers: golang-dev_googlegroups.com,
>>
>> Message:
>> Hello golang-dev@googlegroups.com,
>>
>> I'd like you to review this change to
>> https://code.google.com/p/go/
>>
>>
>> Description:
>> os: avoid panic when testing errors on Plan 9
>>
>> Please review this at http://codereview.appspot.com/6017043/
>>
>> Affected files:
>>  M src/pkg/os/error_plan9.go
>>
>>
>> Index: src/pkg/os/error_plan9.go
>> ===================================================================
>> --- a/src/pkg/os/error_plan9.go
>> +++ b/src/pkg/os/error_plan9.go
>> @@ -5,6 +5,9 @@
>>  package os
>>
>>  func isExist(err error) bool {
>> +       if err == nil {
>> +               return false
>> +       }
>>        if pe, ok := err.(*PathError); ok {
>>                err = pe.Err
>>        }
>> @@ -12,6 +15,9 @@
>>  }
>>
>>  func isNotExist(err error) bool {
>> +       if err == nil {
>> +               return false
>> +       }
>>        if pe, ok := err.(*PathError); ok {
>>                err = pe.Err
>>        }
>> @@ -19,6 +25,9 @@
>>  }
>>
>>  func isPermission(err error) bool {
>> +       if err == nil {
>> +               return false
>> +       }
>>        if pe, ok := err.(*PathError); ok {
>>                err = pe.Err
>>        }
>>
>>
>

[ality, 2012-04-17 00:36:11]

*** Submitted as http://code.google.com/p/go/source/detail?r=99b8351bfb9e ***

os: avoid panic when testing errors on Plan 9

R=golang-dev, bradfitz, akumar
CC=golang-dev
http://codereview.appspot.com/6017043