Re: MSVC patch

2005-02-01 Thread John Levon
On Mon, Jan 31, 2005 at 02:23:54PM +0100, Lars Gullik Bj?nnes wrote:

 This version of is_readonly is racy. So for win32 we should try 

All versions of is_readonly() are racy, it's implicit in the API.

regards
john


Re: MSVC patch

2005-02-01 Thread Lars Gullik Bjønnes
John Levon [EMAIL PROTECTED] writes:

| On Mon, Jan 31, 2005 at 02:23:54PM +0100, Lars Gullik Bj?nnes wrote:

 This version of is_readonly is racy. So for win32 we should try 

| All versions of is_readonly() are racy, it's implicit in the API.

Not if checking readonly is the only thing you want to do?

We shouldn't have to call readonly a lot anyway. The only place where
I think it is needed is for informing the user that the loaded file
is read only. For writing, we should just try to write.. and fail if
we cannot do so.

-- 
Lgb



Re: MSVC patch

2005-02-01 Thread John Levon
On Tue, Feb 01, 2005 at 11:46:17PM +0100, Lars Gullik Bj?nnes wrote:

 We shouldn't have to call readonly a lot anyway. The only place where
 I think it is needed is for informing the user that the loaded file
 is read only. For writing, we should just try to write.. and fail if
 we cannot do so.

Yep. Really we're informing them this file was at least one read only
at some point, so it might be why, but I agree it's not worthwhile to
use a race-free interface.

john


Re: MSVC patch

2005-02-01 Thread John Levon
On Mon, Jan 31, 2005 at 02:23:54PM +0100, Lars Gullik Bj?nnes wrote:

> This version of is_readonly is racy. So for win32 we should try 

All versions of is_readonly() are racy, it's implicit in the API.

regards
john


Re: MSVC patch

2005-02-01 Thread Lars Gullik Bjønnes
John Levon <[EMAIL PROTECTED]> writes:

| On Mon, Jan 31, 2005 at 02:23:54PM +0100, Lars Gullik Bj?nnes wrote:
>
>> This version of is_readonly is racy. So for win32 we should try 
>
| All versions of is_readonly() are racy, it's implicit in the API.

Not if checking readonly is the only thing you want to do?

We shouldn't have to call readonly a lot anyway. The only place where
I think it is "needed" is for informing the user that the loaded file
is read only. For writing, we should just try to write.. and fail if
we cannot do so.

-- 
Lgb



Re: MSVC patch

2005-02-01 Thread John Levon
On Tue, Feb 01, 2005 at 11:46:17PM +0100, Lars Gullik Bj?nnes wrote:

> We shouldn't have to call readonly a lot anyway. The only place where
> I think it is "needed" is for informing the user that the loaded file
> is read only. For writing, we should just try to write.. and fail if
> we cannot do so.

Yep. Really we're informing them "this file was at least one read only
at some point, so it might be why", but I agree it's not worthwhile to
use a race-free interface.

john


Re: MSVC patch

2005-01-31 Thread Angus Leeming
Asger Ottar Alstrup wrote:

 And for sure, it worked the first time.
 Anyway, here is the patch.

Asger, could you commit this to the development/Win32 directory. I'm off 
to the Alps for a week on Friday evening so my hacking-on-LyX time is 
running out fast. It'll be easier to pick up where I left off if this 
patch is in the repository.

-- 
Angus



Re: MSVC patch

2005-01-31 Thread Asger Ottar Alstrup
Angus Leeming wrote:
Asger, could you commit this to the development/Win32 directory. I'm off 
to the Alps for a week on Friday evening so my hacking-on-LyX time is 
running out fast. It'll be easier to pick up where I left off if this 
patch is in the repository.
Done. Elias just woke up, so I'm out of here.
Take care,
Asger


Re: MSVC patch

2005-01-31 Thread Lars Gullik Bjønnes
Asger Ottar Alstrup [EMAIL PROTECTED] writes:

| And for sure, it worked the first time.

| Anyway, here is the patch.

| It asserts on start-up when we pass a Unix-style path to
| boost::filesystem in development/Win32/package.C, but you can just
| ignore that, and it will work fine.

??

Comment:

 bool is_readonly(path const  ph)
 {
-#ifdef BOOST_POSIX
return is_readable(ph)  !is_writable(ph);
-#endif
 }


This version of is_readonly is racy. So for win32 we should try 
to avoid this. An non-racy version would look like this?  (right?)

bool is_readonly(patch const  ph)
{
DWORD const attr = ::GetFileAttributes(ph.string().c_str());
return (attr != INVALID_FILE_ATTRIBUTES
 (attr  FILE_ATTRIBUTE_READONLY));
}

-- 
Lgb



Re: MSVC patch

2005-01-31 Thread Asger Ottar Alstrup
Lars Gullik Bjønnes wrote:
This version of is_readonly is racy. So for win32 we should try 
to avoid this. An non-racy version would look like this?  (right?)

bool is_readonly(patch const  ph)
{
DWORD const attr = ::GetFileAttributes(ph.string().c_str());
return (attr != INVALID_FILE_ATTRIBUTES
 (attr  FILE_ATTRIBUTE_READONLY));
}
Yeah, that should work.
Regards,
Asger


Re: MSVC patch

2005-01-31 Thread Angus Leeming
Asger Ottar Alstrup wrote:

> And for sure, it worked the first time.
> Anyway, here is the patch.

Asger, could you commit this to the development/Win32 directory. I'm off 
to the Alps for a week on Friday evening so my hacking-on-LyX time is 
running out fast. It'll be easier to pick up where I left off if this 
patch is in the repository.

-- 
Angus



Re: MSVC patch

2005-01-31 Thread Asger Ottar Alstrup
Angus Leeming wrote:
Asger, could you commit this to the development/Win32 directory. I'm off 
to the Alps for a week on Friday evening so my hacking-on-LyX time is 
running out fast. It'll be easier to pick up where I left off if this 
patch is in the repository.
Done. Elias just woke up, so I'm out of here.
Take care,
Asger


Re: MSVC patch

2005-01-31 Thread Lars Gullik Bjønnes
Asger Ottar Alstrup <[EMAIL PROTECTED]> writes:

| And for sure, it worked the first time.
>
| Anyway, here is the patch.
>
| It asserts on start-up when we pass a Unix-style path to
| boost::filesystem in development/Win32/package.C, but you can just
| ignore that, and it will work fine.

??

Comment:

 bool is_readonly(path const & ph)
 {
-#ifdef BOOST_POSIX
return is_readable(ph) && !is_writable(ph);
-#endif
 }


This version of is_readonly is racy. So for win32 we should try 
to avoid this. An non-racy version would look like this?  (right?)

bool is_readonly(patch const & ph)
{
DWORD const attr = ::GetFileAttributes(ph.string().c_str());
return (attr != INVALID_FILE_ATTRIBUTES
&& (attr & FILE_ATTRIBUTE_READONLY));
}

-- 
Lgb



Re: MSVC patch

2005-01-31 Thread Asger Ottar Alstrup
Lars Gullik Bjønnes wrote:
This version of is_readonly is racy. So for win32 we should try 
to avoid this. An non-racy version would look like this?  (right?)

bool is_readonly(patch const & ph)
{
DWORD const attr = ::GetFileAttributes(ph.string().c_str());
return (attr != INVALID_FILE_ATTRIBUTES
&& (attr & FILE_ATTRIBUTE_READONLY));
}
Yeah, that should work.
Regards,
Asger