SF.net SVN: harbour-project:[16349] trunk/harbour
snai...@users.sourceforge.net
http://harbour-project.svn.sourceforge.net/harbour-project/?rev=16349&view=rev
Author: snaiperis
Date: 2011-02-22 10:27:49 +0000 (Tue, 22 Feb 2011)
Log Message:
-----------
2011-02-22 12:27 UTC+0200 Mindaugas Kavaliauskas (dbtopas/at/dbtopas.lt)
* harbour/src/rtl/fscopy.c
* added runtime error for HB_FCOPY() bad param types only
* made a function HB_FCOPY() a pure file system function
does not dependent on _SET_DEFAULT setting
; File open mode is not altered a avoid any influence on OS/2
behaviour if source and destination files are the same (see
ChangeLog entry at 2009-11-24 16:57). I guess, hb_fsExpOpen()
still can be changed to hb_fsOpen() with FO_EXCLUSIVE,
FO_CREAT, and FO_TRUNC but I'm unable to test it, so, I leave
current code.
Modified Paths:
--------------
trunk/harbour/ChangeLog
trunk/harbour/src/rtl/fscopy.c
This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.
Viktor Szakáts
Pls revert it, or I'll do in next commit.
HB_FCOPY() should never issue an RTE just like FRENAME()
and FERASE() never does. As for the FXO_DEFAULT, pls
either change the whole function to not use ExtOpen, or add
back FXO_DEFAULT to work consistently.
Viktor
Mindaugas Kavaliauskas
On 2011.02.22 12:49, Viktor Szakáts wrote:
> Pls revert it, or I'll do in next commit.
>
> HB_FCOPY() should never issue an RTE just like FRENAME()
> and FERASE() never does.
Can you explain why HB_FCOPY(.T., 12345) or HB_FCOPY(DATE(), {=>})
should not RTE???
> As for the FXO_DEFAULT, pls
> either change the whole function to not use ExtOpen, or add
> back FXO_DEFAULT to work consistently.
Consistently with that? FXO_DEFAULT is only one of flags that is
supported in hb_fsExtOpen(). I left hb_fsExtOpen() for the only reason,
it uses share locks and these locks can be the core of OS/2 hacking. Now
OS/2 hacking remains (you've reverted all our yesterday commit for it)
and function is consistent with other file system functions - i.e., it
does not pay attention to _SET_DEFAULT.
Viktor, I really do not understand that is your goal. Sometimes I feel
you insist on some Harbour behaviour just to not show for others that
you've changed your opinion. If this battle for no runtime error in case
of HB_FCOPY(.T., 12345) and _SET_DEFAULT is important (though, yesterday
your words was "You're right it'd would be better to avoid mixing
_SET_DEFAULT into the picture"), can we have a function HB_FILECOPY() in
Harbour without side effects made do some unpredictable your decisions?
Regards,
Mindaugas
Viktor Szakáts
>> and FERASE() never does.
>
> Can you explain why HB_FCOPY(.T., 12345) or HB_FCOPY(DATE(), {=>}) should
> not RTE???
Check FRENAME() and FERASE().
>> As for the FXO_DEFAULT, pls
>> either change the whole function to not use ExtOpen, or add
>> back FXO_DEFAULT to work consistently.
>
> Consistently with that? FXO_DEFAULT is only one of flags that is supported
> in hb_fsExtOpen(). I left hb_fsExtOpen() for the only reason, it uses share
> locks and these locks can be the core of OS/2 hacking. Now OS/2 hacking
> remains (you've reverted all our yesterday commit for it) and function is
> consistent with other file system functions - i.e., it does not pay
> attention to _SET_DEFAULT.
Instead of solving the problem properly, you just deleted
_one_ flag which you don't seem to like for some reason.
You ignored the rest of the issue because you're not willing
to deal with them because they are OS/2 related.
Please fix it fully instead of tweaking it to your liking. By
removing FXO_DEFAULT, you just altered behavior so it now
differs from both FRENAME()/FERASE() and both __COPYFILE(),
IOW you've added yet another kind of behavior inside the
same circle of function. Which is not good.
> Viktor, I really do not understand that is your goal. Sometimes I feel you
> insist on some Harbour behaviour just to not show for others that you've
> changed your opinion. If this battle for no runtime error in case of
> HB_FCOPY(.T., 12345) and _SET_DEFAULT is important (though, yesterday your
> words was "You're right it'd would be better to avoid mixing _SET_DEFAULT
> into the picture"), can we have a function HB_FILECOPY() in Harbour without
> side effects made do some unpredictable your decisions?
Unpredictable in what way? Now you've added a third kind of
behavior for the same family of functions, moreover in two
different ways. What was originally committed (3 years ago
BTW) was perfectly consistent and reasoned. Later it was
modified for OS/2, which may need a fix, but that fix should
be proper, not just removing some random flags you don't like.
Pls add HB_FILECOPY() to your own local project, if you are
not willing to fix HB_FCOPY() to use pure hb_fsOpen() or you
don't like it's FRENAME()/FERASE()-like RTE behavior.
Viktor
Mindaugas Kavaliauskas
On 2011.02.22 13:29, Viktor Szakáts wrote:
>>> HB_FCOPY() should never issue an RTE just like FRENAME()
>>> and FERASE() never does.
>>
>> Can you explain why HB_FCOPY(.T., 12345) or HB_FCOPY(DATE(), {=>}) should
>> not RTE???
>
> Check FRENAME() and FERASE().
OK, I see you point of view. FRENAME() and FERASE() do not RTE on wrong
parameters. But these functions are not harbour no HB_ prefix and we
should keep this behaviour (no RTE) for Clipper compatibility. BWT,
FOPEN() do RTE! So, even in Clipper it was not a rule that functions do
not RTE!
AFAIU, the idea of Harbour functions with HB_ prefix is that WE can
choose how it works and we are not bound to Clipper's legacy. In some
cases we even created our own HB_*() functions which are analogues to
Clipper functions, but have more preferred behaviour, and left original
Clipper functions as it was. So, why in case of HB_FCOPY() we should do
in a different way?
I see RTE in case of HB_FCOPY(.T., 12345) a preferred action instead of
silent return file not found. Don't you see a advantage in this? Why for
HB_* function we can not choose this behaviour?
> Instead of solving the problem properly, you just deleted
> _one_ flag which you don't seem to like for some reason.
> You ignored the rest of the issue because you're not willing
> to deal with them because they are OS/2 related.
>
> Please fix it fully instead of tweaking it to your liking. By
> removing FXO_DEFAULT, you just altered behavior so it now
> differs from both FRENAME()/FERASE() and both __COPYFILE(),
> IOW you've added yet another kind of behavior inside the
> same circle of function. Which is not good.
Solving what problem? If we forget about RTE issue, there are two more
distinct things in HB_FCOPY():
1) usage of _SET_DEFAULT
2) OS/2 hack related to open attributes and file locking
The first one is managed by FXO_DEFAULTS.
The second by FXO_EXCLUSIVE, FXO_TRUNCATE and FXO_SHARELOCK.
These two things are completely independent actions, but both of it are
implemented in one function hb_fsExtOpen(). Actually, hb_fsExtOpen() has
even more capabilities like adding default extension, but we use only
teo of then in case if HB_FCOPY().
AFAIK understand you want to leave OS/2 behaviour but remove
_SET_DEFAULT dependence for file system function. So, the road to reach
this is do not use FXO_DEFAULT and use FXO_SHARELOCK.
What wrong in this?! Does FXO_DEFAULT should always be set? If yes, then
why do we have this parameter at all? Maybe we need to hardcode it into
hb_fsExtOpen() if you find something wrong in calling hb_fsExtOpen()
without it.
What solutions do you see to make this function work as expected?
Simple hb_fsOpen()/hb_fsCreate() is not enough, because it will not have
OS/2 behaviour hack. Of course we can inline hb_fsExtOpen() code, delete
some unused parts from it. This will have the behaviour as expected
without hb_fsExtOpen() function call, with duplicated hb_fsExtOpen()
code. Do you see this solution to be a good and "not tweak"?
> Unpredictable in what way?
Unperdictable in a way, that you in general agree that RTE is a good to
find bugs (instead of silent return .F.) in case of wrong paramters
types are passed, but for function HB_FCOPY() you have an opposite opinion.
Unpredictable in way, that you do not like _SET_DEFAULT interaction with
file system copy function, but you add FXO_DEFAULTS.
> Now you've added a third kind of
> behavior for the same family of functions, moreover in two
> different ways. What was originally committed (3 years ago
> BTW) was perfectly consistent and reasoned.
No, you added a new kind of behaviour for file system function by adding
FXO_DEFAULTS. All file system function do not depend on _SET_DEFAULT,
but HB_FCOPY() depends. Maybe it was 3 years ago, but I see no problem
to fix it after some time.
> Later it was
> modified for OS/2, which may need a fix, but that fix should
> be proper, not just removing some random flags you don't like.
FXO_DEFAULT flag removing is not related to OS/2. FXO_DEFAULT is not
random and it shows that _SET_DEFAULT should be used, i.e., what we are
trying to avoid. And this is not because I don't like, but because you
don't like it also ("You're right it'd would be better to avoid mixing
_SET_DEFAULT into the picture").
> Pls add HB_FILECOPY() to your own local project, if you are
> not willing to fix HB_FCOPY() to use pure hb_fsOpen()
To have OS/2 hack we will need to add all locking that exists in
hb_fsExtOpen(). This is will be equivalent to inlineing hb_fsExtOpen()
(with some unused code deleted) in hb_fsCopy(). Do you want this to be done?
Regards,
Mindaugas
Viktor Szakáts
>
> OK, I see you point of view. FRENAME() and FERASE() do not RTE on wrong
> parameters. But these functions are not harbour no HB_ prefix and we should
> keep this behaviour (no RTE) for Clipper compatibility. BWT, FOPEN() do RTE!
> So, even in Clipper it was not a rule that functions do not RTE!
It's irrelevant how other Clipper functions work. There is no
generic rule for that in Clipper. There is also no generic rule
for it in Harbour functions. With HB_FCOPY() my only goal was
to make them similar to FRENAME() and FERASE() because
they are the closest in functionality. It also doesn't matter if
HB_ prefix is used here, as this is the only way to add new
functions and by itself it doesn't mean we must follow any
generic HB_-rule for error handling, simply because there is
no such generic rule.
> AFAIU, the idea of Harbour functions with HB_ prefix is that WE can choose
> how it works and we are not bound to Clipper's legacy. In some cases we even
> created our own HB_*() functions which are analogues to Clipper functions,
> but have more preferred behaviour, and left original Clipper functions as it
> was. So, why in case of HB_FCOPY() we should do in a different way?
> I see RTE in case of HB_FCOPY(.T., 12345) a preferred action instead of
> silent return file not found. Don't you see a advantage in this? Why for
> HB_* function we can not choose this behaviour?
The general rule is that we use HB_ prefix for _all_ Harbour extensions.
There is no general rule that would force us to use any specific
error handling method just because the function is an extension,
marked with HB_ prefix.
> Solving what problem? If we forget about RTE issue, there are two more
> distinct things in HB_FCOPY():
> 1) usage of _SET_DEFAULT
> 2) OS/2 hack related to open attributes and file locking
This is _one_ problem, namely that HB_FCOPY() now uses
hb_fsExtOpen(), instead of hb_fsOpen()/hb_fsCreate().
hb_fsExtOpen() has other differences compared to "pure"
open/create functions, not just FXO_DEFAULTS. Pls explain
why FXO_DEFAULTS is more relevant here than the rest
of these differences.
> These two things are completely independent actions, but both of it are
> implemented in one function hb_fsExtOpen(). Actually, hb_fsExtOpen() has
> even more capabilities like adding default extension, but we use only teo of
> then in case if HB_FCOPY().
>
> AFAIK understand you want to leave OS/2 behaviour but remove _SET_DEFAULT
> dependence for file system function. So, the road to reach this is do not
> use FXO_DEFAULT and use FXO_SHARELOCK.
>
> What wrong in this?! Does FXO_DEFAULT should always be set? If yes, then why
> do we have this parameter at all? Maybe we need to hardcode it into
> hb_fsExtOpen() if you find something wrong in calling hb_fsExtOpen() without
> it.
Pls give it a wider look. __COPYFILE() uses hb_fsExtOpen with all these
flags, other functions use pure hb_fsOpen()/hb_fsCreate(). Now you try to invent
a third category in these closely related functions: hb_fsExtOpen
without FXO_DEFAULT.
Why would that be good? (besides saving development time)
> What solutions do you see to make this function work as expected?
Implement file copy using hb_fsOpen()/hb_fsCreate()[/hb_fsLock()]
while keeping it compatible with OS/2.
> Simple hb_fsOpen()/hb_fsCreate() is not enough, because it will not have
> OS/2 behaviour hack. Of course we can inline hb_fsExtOpen() code, delete
> some unused parts from it. This will have the behaviour as expected without
> hb_fsExtOpen() function call, with duplicated hb_fsExtOpen() code. Do you
> see this solution to be a good and "not tweak"?
Yes, this would be the correct solution.
>> Unpredictable in what way?
>
> Unperdictable in a way, that you in general agree that RTE is a good to find
> bugs (instead of silent return .F.) in case of wrong paramters types are
> passed, but for function HB_FCOPY() you have an opposite opinion.
There is no generic rule for throwing RTE or not. Pls look around
our 6000 functions, or just the HB_ ones. Yes, I prefer RTE for wrong
parameters in general, but in this case it's much more important to work
like well-known and used FRENAME()/FERASE() because they are
in the exact same functional category.
> Unpredictable in way, that you do not like _SET_DEFAULT interaction with
> file system copy function, but you add FXO_DEFAULTS.
I don't like that you try to introduce yet another way of opening
files instead of fixing the hb_fsExtOpen() problem as a whole.
IOW: FXO_DEFAULTS is not the only extension here.
>> Now you've added a third kind of
>> behavior for the same family of functions, moreover in two
>> different ways. What was originally committed (3 years ago
>> BTW) was perfectly consistent and reasoned.
>
> No, you added a new kind of behaviour for file system function by adding
> FXO_DEFAULTS. All file system function do not depend on _SET_DEFAULT, but
> HB_FCOPY() depends. Maybe it was 3 years ago, but I see no problem to fix it
> after some time.
_I didn't add_ FXO_DEFAULTS, I copied this whole call from __COPYFILE(),
so I didn't add a new way of opening files. __COPYFILE() is pretty similar
in functionality to HB_FCOPY(), so this was least confusing for users.
(of course it would have been even better to identify the exact ExtOpen
feature that makes it work for OS/2, but for me it was not pressing enough. ]
> FXO_DEFAULT flag removing is not related to OS/2. FXO_DEFAULT is not random
> and it shows that _SET_DEFAULT should be used, i.e., what we are trying to
> avoid. And this is not because I don't like, but because you don't like it
> also ("You're right it'd would be better to avoid mixing _SET_DEFAULT into
> the picture").
>
>
>> Pls add HB_FILECOPY() to your own local project, if you are
>> not willing to fix HB_FCOPY() to use pure hb_fsOpen()
>
> To have OS/2 hack we will need to add all locking that exists in
> hb_fsExtOpen(). This is will be equivalent to inlineing hb_fsExtOpen() (with
> some unused code deleted) in hb_fsCopy(). Do you want this to be done?
Yes. Without all extra fluffs going on in hb_fsExtOpen(), like
hb_fsAddSearchPath() calls which is going to happen even if
you delete FXO_DEFAULTS.
This will also help to precisely identify what's needed for OS/2
to avoid the truncation problem when source and destination
is the same.
Viktor
Maurilio Longo
can I help in any way? Which hack is needed by OS/2?
Maurilio.
--
__________
| | | |__| Maurilio Longo
|_|_|_|____| farmaconsult s.r.l.
Viktor Szakáts
On Tue, Feb 22, 2011 at 2:11 PM, Maurilio Longo
<maurili...@libero.it> wrote:
> Viktor,
>
> can I help in any way? Which hack is needed by OS/2?
Thanks for jumping in. I've fixed HB_FCOPY() for OS/2
a while ago by using hb_fsExtOpen() (duplicating the method
used in __COPYFILE()), to fix the problem where the file
got truncated to zero length if it was used for both source
and destination. Maybe you remember it better, since you
reported it.
Anyways, now Mindaugas restored initial method with
a little twist, so it'd be good to see how it works on OS/2.
Viktor
Mindaugas Kavaliauskas
On 2011.02.22 14:42, Viktor Szakáts wrote:
> It's irrelevant how other Clipper functions work. There is no
> generic rule for that in Clipper. There is also no generic rule
> for it in Harbour functions. With HB_FCOPY() my only goal was
> to make them similar to FRENAME() and FERASE() because
> they are the closest in functionality. It also doesn't matter if
> HB_ prefix is used here, as this is the only way to add new
> functions and by itself it doesn't mean we must follow any
> generic HB_-rule for error handling, simply because there is
> no such generic rule.
I would not agree that we do not have a general rule to use RTE instead
of silent return. But I see you prefer consistency with Clipper
functions more than this. OK, at least I see you point of view. Though,
I have the different. It would be nice to see that other user think.
>> Solving what problem? If we forget about RTE issue, there are two more
>> distinct things in HB_FCOPY():
>> 1) usage of _SET_DEFAULT
>> 2) OS/2 hack related to open attributes and file locking
>
> This is _one_ problem, namely that HB_FCOPY() now uses
> hb_fsExtOpen(), instead of hb_fsOpen()/hb_fsCreate().
>
> hb_fsExtOpen() has other differences compared to "pure"
> open/create functions, not just FXO_DEFAULTS. Pls explain
> why FXO_DEFAULTS is more relevant here than the rest
> of these differences.
As I mentioned before hb_fsExtOpen() has MANY functionality.
FXO_DEFAULTS is one of them. It is independent from others! And we do
not want _SET_DEFAULT to influence filesystem function, so, we do not
add this flags. The flags are implemented for that purpose. We set only
the flags we need. In our case we do not need FXO_DEFAULT, so we do not
set it.
> Pls give it a wider look. __COPYFILE() uses hb_fsExtOpen with all these
> flags, other functions use pure hb_fsOpen()/hb_fsCreate(). Now you try to invent
> a third category in these closely related functions: hb_fsExtOpen
> without FXO_DEFAULT.
> Why would that be good? (besides saving development time)
Number of used flags depends on the functionality we want to have. If we
would like default extension we will use the "fourth category" if we
need error we will use "fifth category" by passing pError parameter. In
this case we don't want _SET_DEFAULT and want (at lease some of as want)
OS/2, so, we do not pass FXO_DEFAULTS and pass FXO_SHARELOCK. What could
be more simple?
>> What solutions do you see to make this function work as expected?
>
> Implement file copy using hb_fsOpen()/hb_fsCreate()[/hb_fsLock()]
> while keeping it compatible with OS/2.
So, we will need to replicate a locking part of hb_fsExtOpen(). We can
do this. But look at this code:
#if defined( HB_USE_SHARELOCKS )
if( hFile != FS_ERROR && uiExFlags & FXO_SHARELOCK )
{
#if defined( HB_USE_BSDLOCKS )
int iLock;
if( ( uiFlags & ( FO_READ | FO_WRITE | FO_READWRITE ) ) == FO_READ ||
( uiFlags & ( FO_DENYREAD | FO_DENYWRITE | FO_EXCLUSIVE ) )
== 0 )
iLock = LOCK_SH | LOCK_NB;
else
iLock = LOCK_EX | LOCK_NB;
hb_vmUnlock();
iLock = flock( hFile, iLock );
hb_vmLock();
if( iLock != 0 )
#else
HB_USHORT uiLock;
if( ( uiFlags & ( FO_READ | FO_WRITE | FO_READWRITE ) ) == FO_READ ||
( uiFlags & ( FO_DENYREAD | FO_DENYWRITE | FO_EXCLUSIVE ) )
== 0 )
uiLock = FL_LOCK | FLX_SHARED;
else
uiLock = FL_LOCK | FLX_EXCLUSIVE;
if( !hb_fsLockLarge( hFile, HB_SHARELOCK_POS, HB_SHARELOCK_SIZE,
uiLock ) )
#endif
{
hb_fsClose( hFile );
hFile = FS_ERROR;
/*
* fix for neterr() support and Clipper compatibility,
* should be revised with a better multi platform solution.
*/
hb_fsSetError( ( uiExFlags & FXO_TRUNCATE ) ? 5 : 32 );
}
This code will be required to include. Actually, more than this.
Well..., twice. Why we have to duplicate all this code instead of
calling function that already implements it and asking to use this code
with a flag FXO_SHARELOCK?
>> Simple hb_fsOpen()/hb_fsCreate() is not enough, because it will not have
>> OS/2 behaviour hack. Of course we can inline hb_fsExtOpen() code, delete
>> some unused parts from it. This will have the behaviour as expected without
>> hb_fsExtOpen() function call, with duplicated hb_fsExtOpen() code. Do you
>> see this solution to be a good and "not tweak"?
>
> Yes, this would be the correct solution.
No, Viktor. Now we have it one place instead of two copies. This will
be a source of problems then we will need to change code. We will need
to keep it in sync.
>> Unpredictable in way, that you do not like _SET_DEFAULT interaction with
>> file system copy function, but you add FXO_DEFAULTS.
>
> I don't like that you try to introduce yet another way of opening
> files instead of fixing the hb_fsExtOpen() problem as a whole.
> IOW: FXO_DEFAULTS is not the only extension here.
Maybe I'm not fixing OS/2 hack. Actually I do not think it should be
included at all (see P.S.). But I do fix _SET_DEFAULTS issue. If one of
two issues solved, the code should be fixed without your argument fix
both or none. Otherwise this means we should not commit any fixes to
Harbour if it does not fix all issues that can be found in future.
> Yes. Without all extra fluffs going on in hb_fsExtOpen(), like
> hb_fsAddSearchPath() calls which is going to happen even if
> you delete FXO_DEFAULTS.
No, there is no extras included if we do not pass extra flags.
hb_fsAddSearchPath() will be used if you use pPaths parameter. We are
not using it, so, we are not influenced by this behaviour.
> You didn't even wait for me to answer to commit this
Sorry, Viktor, I've just tried to act the way you act. You reverted my
yesterday code without answering to letter.
Regards,
Mindaugas
P.S. the reason why I looked at HB_FCOPY() source was:
PROC MAIN()
LOCAL hFile
HB_MEMOWRIT("test242.txt", "abc")
hFile := FOPEN("test242.txt", 64) // shared
HB_FLOCK(hFile, 1024, 1)
? HB_FCOPY("test242.txt", "test242_copy.txt")
? FERROR()
? LEN(HB_MEMOREAD("test242_copy.txt"))
FCLOSE(hFile)
RETURN
The code on Windows prints:
.T.
0
0
I.e., no error returned, no file copied (empty file created). On Linux
it copies file correctly. The reason of not copying file is that copying
is done in 16384 bytes buffers. Windows returns locking error on
ReadFile() request for 16384 bytes if byte number 1024 is locked, even
if file actually contains 3 bytes only.
I'm not insisting on Windows specific hack as we have OS/2 hack. We will
never be able to hide all system OS differencies. So, even pure
hb_fsOpen()/hb_fsCreate() is a hacky solution. We should implement a
pure OS file copy function like CopyFile() in Win32, if we want to avoid
hacks. Only this way we will have a pure file system copy function.
Przemysław Czerpak
Hi,
> It's irrelevant how other Clipper functions work. There is no
> generic rule for that in Clipper. There is also no generic rule
> for it in Harbour functions. With HB_FCOPY() my only goal was
> to make them similar to FRENAME() and FERASE() because
> they are the closest in functionality. It also doesn't matter if
> HB_ prefix is used here, as this is the only way to add new
> functions and by itself it doesn't mean we must follow any
> generic HB_-rule for error handling, simply because there is
> no such generic rule.
Anyhow for new functions I'd suggest to generate RTE for
completely wrong parameters. It helps to keep the code clean
and introduce some extensions which depends on type of passed
parameters, i.e. it's much safer to add support for sth like:
hFiles := { "file1.dat" => "bak/file2.dat", ;
"file2.dat" => "bak/file2.dat", ;
"file3.dat" => "bak/file3.dat" }
hb_fcopy( hFiles )
if current version generates RTE for such code because we do not
have to worry about possible interactions with some silent bugs
in user code so RTE for completely wrong parameters is always
preferred behavior for me.
> hb_fsExtOpen() has other differences compared to "pure"
> open/create functions, not just FXO_DEFAULTS. Pls explain
> why FXO_DEFAULTS is more relevant here than the rest
> of these differences.
So we have problem now because we do not have file copy
function which like FOPEN(), FCREATE(), FRENAME() and
FERASE() ignores _SET_DEFAULT so can be safely used with
above functions and programmer is sure that his code operates
on the same files regardless of _SET_DEFAULT settings.
I thing that it should be the main difference between
__COPYFILE() and HB_FCOPY(). So far I haven't found any
other reasonable arguments (important enough) to keep both
functions.
FXO_SHARELOCK is important if some code depends on exclusive
access to destination files so this is good reason to use
hb_fsExtOpen() instead of hb_fsOpen()/hb_fsCreate().
Anyhow I would like to note that there is also one extremely
important platform difference which causes that we should try
to use hb_file*() functions (hb_fileExtOpen()) instead of
hb_fs*() if it's possible and looks that you do not know
about it:
In POSIX systems all file range locks are bound with file
i-node not file handle and are removed when _any_ file handle
to given i-node used by the process is closed.
It means that if you make:
use data.dbf shared
if dbrlock(1) .and. ;
dbrlock(2) .and. ;
dbrlock(10)
// now three records are locked
fclose( fopen( "data.dbf" ) )
// now all locks are released in hidden way by close() on file
// file handle which points to the same i-node as file used by
// current WA so other stations can make flock() or even
// use data.dbf exclusive
// because also FXO_SHARELOCK locks are removed (on systems which
// do not use BSD locks (HB_USE_BSDLOCKS) to emulate DOS DENY flacks)
// this function only removes information about locks from internal
// WA structures but real file range locks were removed earlier by
// close() C function inside C code.
dbUnlock()
endif
Inside hb_file*() API I implemented code which recognize that file is
reused in *nix systems so already open OS handle is also reused and we
do not have danger close() call which removes locks set on other handles
inside hb_fileClose().
In the future we should think about extending hb_file*() API so it can
be used in all places instead of current hb_fs*() ones, i.e. to make
safe backup creation for already open .DBF files.
Such extenssion should introduce two new flags:
FXO_BUFFERED // read/write buffers so it can be used efficiently
// instead of fopen()/fwrite()/fread()/fclose()
FXO_SEEKSET // emulate thread local read/write file offset
// with hb_fileSeek()/hb_fileRead()/hb_fileWrite()
// functions. Replaceable IO systems which do not
// provide above three functions should refuse to
// open file when this flag is used
and at least three new methods:
Seek()
Read()
Write()
This in the future. Now we should try to use hb_file*() functions
in all places where current API is enough, i.e. inside __COPYFILE()
and HB_FCOPY().
BTW I suggest to extend the copy buffer to at least 64KB (the biggest
cluster size in FAT) to not introduce unnecessary overhead.
best regards,
Przemek
Mindaugas Kavaliauskas
On 2011.02.22 18:29, Viktor Szakáts wrote:
> If this is useful section which can be used as is, it can
> be put in an internal functions. Pls don't tell me if this is
> the end of the world, it is surely not a reason to use hacks
> instead of proper solution.
It is already put in function. Name of this function is hb_fsExtOpen().
And this is the proper solution.
> So instead of "duplicating code", you rather break
> functionality or create half-assed solution fixing the
> only sub-problem you happen to be worried about.
First of all I do not break. Second, I do not duplicate not because it
is hard for me to do copy paste, but because I see duplication a wrong
solution, that should be fixed by using hb_fsExtOpen().
> Now I plan to revert your latest change because I see that you
> are apparently not concerned by the big picture, just your
> own local _SET_DEFAULT problem, so the discussion is
> completely pointless.
_SET_DEFAULT is not my problem, I just wanted to make all file system
function be consistent. There is one thing we agree on, the discussion
is pointless. The sad side of this is that pointless discussions makes
me less motivated to do any commits I was planned to do before 2.x
release. Please revert to anything you want.
Regards,
Mindaugas
Viktor Szakáts
>> are apparently not concerned by the big picture, just your
>> own local _SET_DEFAULT problem, so the discussion is
>> completely pointless.
>
> _SET_DEFAULT is not my problem, I just wanted to make all file system
> function be consistent. There is one thing we agree on, the discussion is
> pointless. The sad side of this is that pointless discussions makes me less
> motivated to do any commits I was planned to do before 2.x release. Please
> revert to anything you want.
The moral of the story is next time pls ask before you
shoot incompatible patches into core, thus maybe the
whole thing can be discussed with much less frustration,
and as a result we have a better end-result with much less
waste of time. It's also a good idea to inspect commit history
before trying to reinvent existing features. It's also good idea
to publish actual problem (which you unfortunately did only
one e-mail ago, at pretty much the end of one day struggle),
so others can understand the problems you're having.
[ BTW for your problem, it was not necessary to modify
_SET_DEFAULT and RTE behavior, so it was unnecessary
to push these matters to the max. ]
If your conclusion is to not commit more, what can I do.
We're in feature freeze anyway, so maybe it's better anyway
to wait until release. But maybe I'm also going defensive
and there won't be any release. Funny.
Viktor
Ranier
> IMO the problem is Viktor live in clipper age and thinks he owns the
> harbor!
> Clipper is dead.
>
> Mindaugas thank you for the efforts to improve the harbor.
>
> Best regards,
>
> Ranier Vilela
>
Hi,
Sorry typo in name, Harbour.
Ranier Vilela
Viktor Szakáts
>> open/create functions, not just FXO_DEFAULTS. Pls explain
>> why FXO_DEFAULTS is more relevant here than the rest
>> of these differences.
>
> As I mentioned before hb_fsExtOpen() has MANY functionality. FXO_DEFAULTS is
> one of them. It is independent from others! And we do not want _SET_DEFAULT
> to influence filesystem function, so, we do not add this flags. The flags
> are implemented for that purpose. We set only the flags we need. In our case
> we do not need FXO_DEFAULT, so we do not set it.
No, because you will introduce inconsistent behavior in
the core domain.
You can do whatever you want in your own local lib when
calling hb_fsExtOpen(), but inside core, we need to restrict
ourself to give least confusing behavior.
> Number of used flags depends on the functionality we want to have. If we
> would like default extension we will use the "fourth category" if we need
> error we will use "fifth category" by passing pError parameter. In this case
> we don't want _SET_DEFAULT and want (at lease some of as want) OS/2, so, we
> do not pass FXO_DEFAULTS and pass FXO_SHARELOCK. What could be more simple?
Question is not what is simplest for you to implement,
but what is best for core.
If this is useful section which can be used as is, it can
be put in an internal functions. Pls don't tell me if this is
the end of the world, it is surely not a reason to use hacks
instead of proper solution.
>> Yes, this would be the correct solution.
>
> No, Viktor. Now we have it one place instead of two copies. This will be a
> source of problems then we will need to change code. We will need to keep it
> in sync.
So instead of "duplicating code", you rather break
functionality or create half-assed solution fixing the
only sub-problem you happen to be worried about.
Nice.
>> I don't like that you try to introduce yet another way of opening
>> files instead of fixing the hb_fsExtOpen() problem as a whole.
>> IOW: FXO_DEFAULTS is not the only extension here.
>
> Maybe I'm not fixing OS/2 hack. Actually I do not think it should be
> included at all (see P.S.). But I do fix _SET_DEFAULTS issue. If one of two
> issues solved, the code should be fixed without your argument fix both or
> none. Otherwise this means we should not commit any fixes to Harbour if it
> does not fix all issues that can be found in future.
I try to look at the complete picture and your solution is
wrong, you only focus on your own local problem.
> No, there is no extras included if we do not pass extra flags.
> hb_fsAddSearchPath() will be used if you use pPaths parameter. We are not
> using it, so, we are not influenced by this behaviour.
>
>
>> You didn't even wait for me to answer to commit this
>
> Sorry, Viktor, I've just tried to act the way you act. You reverted my
> yesterday code without answering to letter.
First you committed something under active discussion,
apparently without considering anything I wrote or without
understanding (or even reading) my POV. Then I implemented
some stuff you asked for, than spent two hours researching
old commits and the whole history of the function and reverted
my fix along with your because they were wrong and against
my initial idea I've added HB_FCOPY() three years ago in the
first place.
Then you reverted the whole thing as per your idea without
saying a word. Next you tweaked it without waiting for my
answer.
Now I plan to revert your latest change because I see that you
are apparently not concerned by the big picture, just your
own local _SET_DEFAULT problem, so the discussion is
completely pointless.
Viktor
Ranier
Viktor Szakáts
> IMO the problem is Viktor live in clipper age and thinks he owns the harbor!
> Clipper is dead
If your comment would have the smallest bit of
wit in it, maybe I'd argue with it, but apparently
you have no clue about this problem and my
involvement, too.
Thanks for sharing anyway, if it'd be up to you
also Harbour would be dead by now.
Viktor
Ranier
You right about my participation is zero in Harbour.
At least I'm not arguing that a ton of words to make a small correction
in Harbour.
Best regards,
Ranier Vilela
alcisoft
>IMO the problem is Viktor live in clipper age and thinks he owns the
harbor!
>Clipper is dead.
>
>Mindaugas thank you for the efforts to improve the harbor.
Your words disturb harbour-project and his developers.
Very bad thing and very bad words yours.
Congratulations **ALL* harbour developers. You helping us.
Regards
Alex Strickland
> IMO the problem is Viktor live in clipper age and thinks he owns the
> harbor!
> Clipper is dead.
You are just stirring the pot. Mostly the discussion is technical and
has lasted one whole day. In a 6 or 7 year history it is nothing. Viktor
certainly does not live in the "clipper age" and I can only assume that
if you say so, you have not been paying attention, or you simply wish to
troll.
> Mindaugas thank you for the efforts to improve the harbor.
I agree in this at least.
--
Alex
Viktor Szakáts
> Is about political changes, Mindaugas have two commits were revert by
> Viktor, without consent of Mindaugas.
No, first I added patches, which I myself reverted,
and which is clear sign that I don't like it, or that I'm
unsure about it, and Mindaugas's answer was to
restore it without any discussion. Then he reverted
my change again in the middle of discussion apparently
without even reading my answer. Also not extremely
polite.
And to go even further, I'm the initial committer of
HB_FCOPY() and I'm still around, so as a still active
developer (or "owner") of this function it's customary that
anybody who wishes to alter behavior _asks that "owner"_
about it, not vice versa. It'd very strange if I'd need to
ask "consent" from someone who patches my work
without discussion just to be able to restore it.
> Each change (commit) is necessary proof scientific and one war of words to
> convince Viktor about.
What a hypocrisy. So you don't like discussion (or
"academic bullshit" as someone called it on another
list) (on a developers mailing list, which makes it
outright funny!), yet you like the fruits of finding the
best possible solution in Harbour, right?
So what do you propose for us? Finding the best
solution without discussion? Out of thin air? Or
you plan to propose it yourself?
I do believe that any change should be explainable
and need to have some proofs that it makes things
better. That's what makes Harbour better, and that's
why we have developer's list.
If discussion bothers you, don't read it. Easy.
> Obsiosly that I´m grateful toward hard work of Viktor in Harbour.
> Just wish he would consider a little more the work of others, especially
> Mindaugas and Pritpal.
You believe your comments were of any help to
strengthen respect on this list? You tell me about
respect after your words directed to me?
I very much respect Mindaugas's work (perhaps,
if you watch closely - which you don't - you can tell
why hbrun got hbmzip and hbmemio linked in first
round, or hbhttpd promoted as contrib, why I added
new sdd drivers, and I could go on), and I also highly
respect Pritpal's huge efforts and time put into Harbour,
(otherwise I wouldn't be pushing HBQT). They are
both in the top few contributors, this is quite obvious
just by skimming through their commits.
Yet, it won't stop me to try to understand the validity of
any change, or point to any problems (or God forbid
expect respect myself!). I hope others will also point to any
problems in my contributions, what I'm asking for is
discussion if something has any dubious factor in it.
Committing while there is doubt in the air, or no conclusion
is made, is rude (and it's also nice way to wreck a discussion,
like in this case). There is nothing new in this, it worked
the same way since the beginning.
Viktor
Ranier
My comment is not about technical obviosly.
Is about political changes, Mindaugas have two commits were revert by
Viktor, without consent of Mindaugas.
to convince Viktor about.
Obsiosly that I´m grateful toward hard work of Viktor in Harbour.
Just wish he would consider a little more the work of others, especially
Mindaugas and Pritpal.
Best regards,
Ranier Vilela
Viktor Szakáts
Which entitles you perfectly to form an opinion on my
motives (or for anyones' motives who actually do
participate) and how I'm being a Clipper user, and who
knows what else.
> At least I'm not arguing that a ton of words to make a small correction in
> Harbour.
Maybe you should try to find a project where modifications
can go without discussions and critics. This method surely
works out very well on the long run. But why do you use
Harbour if it's apparently so too carefully developed for your
taste (and the price you paid for it)? (No need to answer)
Viktor
Mindaugas Kavaliauskas
It's a pitty that were is no sentence with true in your mail.
On 2011.02.22 19:03, Viktor Szakáts wrote:
> The moral of the story is next time pls ask before you
> shoot incompatible patches into core,
They are not incompatible, and now you've committed in now yourself.
> thus maybe the
> whole thing can be discussed with much less frustration,
> and as a result we have a better end-result with much less
> waste of time.
Of course it would be less frustration if you can have any arguments.
Instead of saying that one more call to hb_fsExtOpen() with difference
parameters is bad. Perhaps you have STR(nI) in STR(aI[ni]), but you do
not say that it is wrong. So, calling hb_fsExtOpen() with and without
FXO_DEFAULTS is also not wrong.
> It's also a good idea to inspect commit history
> before trying to reinvent existing features.
If you are talking about my hb_fsOpen()/hb_fsCreate(), so, it was
inspected, I know I committed the same code as before OS/2 fix/hack. I
just believe you are much more clever person that the words you've wrote
last two days. So, I wanted to test, that is your opinion about this
OS/2 issue. Because it does not seem your words expressed your opinion.
From your reaction I've understood that you want to leave OS/2
dependent code (and I was never against it).
> It's also good idea
> to publish actual problem (which you unfortunately did only
> one e-mail ago, at pretty much the end of one day struggle),
I had no problem with HB_FCOPY(). Neither runtime error nor _SET_DEFAULT
issue is not my problem. But I tent to fix bad code i Harbour even if it
does not touches me.
> [ BTW for your problem, it was not necessary to modify
> _SET_DEFAULT and RTE behavior, so it was unnecessary
> to push these matters to the max. ]
Wrong again. It is not my problem. See, above.
The moral is that this issue is not a Harbour development issue, but
personality development issue. I see it not for the first time on
mailing list, both in "discussions" with me and other developers. Your
problem is that you can not accept you can be wrong. If someone has
different opinion, your first and only goal is prove your are right.
Otherwise your self-image is damaged. After you are a winner, you can
see thing in a clear light, you can change your opinion. This was
clearly showed in last two days. You removed FXO_DEFAULTS yourself (that
was the core question of all this topic), right after I gave up in
"discussion".
You should not to be afraid of changing opinion. It does not make you
less valuable as a person. If you keep this in mind, I'm pretty sure
you'll have a new good experience both in technical discussion and
personal life.
Regards,
Mindaugas
Viktor Szakáts
Thanks for setting the proper tone for a cooled down
discussion right on top.
Your total lack of respect is extraordinary.
>> The moral of the story is next time pls ask before you
>> shoot incompatible patches into core,
>
> They are not incompatible, and now you've committed in now yourself.
After spending few more hours examining the details.
>> thus maybe the
>> whole thing can be discussed with much less frustration,
>> and as a result we have a better end-result with much less
>> waste of time.
>
> Of course it would be less frustration if you can have any arguments.
> Instead of saying that one more call to hb_fsExtOpen() with difference
> parameters is bad. Perhaps you have STR(nI) in STR(aI[ni]), but you do not
> say that it is wrong. So, calling hb_fsExtOpen() with and without
> FXO_DEFAULTS is also not wrong.
So tell me! You're popping up multiple unrelated issues at
one go without doing any research, reasoning or explanation
whatsoever and expect me to know every detail regarding
anything you throw at me? How's that, and how about you doing
some research and trying to pinpoint _actual problems_ or
mistakes in the argument? F.e. "no, there is no other extra
feature in hb_fsExtOpen() besides FXO_SHARELOCKS",
case closed.
> If you are talking about my hb_fsOpen()/hb_fsCreate(), so, it was inspected,
> I know I committed the same code as before OS/2 fix/hack. I just believe you
> are much more clever person that the words you've wrote last two days. So, I
> wanted to test, that is your opinion about this OS/2 issue. Because it does
> not seem your words expressed your opinion. From your reaction I've
> understood that you want to leave OS/2 dependent code (and I was never
> against it).
I don't know what you refer to by me lying to you,
at best I can imagine I didn't spend enough hours trying
to do everybody's homework on my own, this is not
my paying job, so it's possible. That's the point of a
discussion actually.
And yes I'd like to keep OS/2 in working state, because
HB_FCOPY( "myapp.prg", "myapp.prg" ) deleting the file
is actually a problem to take care of. You can call it OS/2
specific hack, but since you also don't know what other
platforms are affected, it's IMO a mistake to call it OS/2
specific hack.
> You should not to be afraid of changing opinion. It does not make you less
> valuable as a person. If you keep this in mind, I'm pretty sure you'll have
> a new good experience both in technical discussion and personal life.
No need to patronize me, especially not regarding my
personal life.
Viktor
Mindaugas Kavaliauskas
On 2011.02.23 13:40, Viktor Szakáts wrote:
>> They are not incompatible, and now you've committed in now yourself.
>
> After spending few more hours examining the details.
I've spent more time on HB_FCOPY() during Windows locking behaviour
analysis.
> expect me to know every detail regarding
> anything you throw at me? How's that, and how about you doing
> some research and trying to pinpoint _actual problems_ or
> mistakes in the argument? F.e. "no, there is no other extra
> feature in hb_fsExtOpen() besides FXO_SHARELOCKS",
> case closed.
1) You do not need to know details in advance. 2) I did some research.
3) I was trying to explain you details for two days. Though, explanation
changes nothing if no one tries to understand this...
> I don't know what you refer to by me lying to you
Have I ever said you are lying? Can you cite me?
>> You should not to be afraid of changing opinion. It does not make you less
>> valuable as a person. If you keep this in mind, I'm pretty sure you'll have
>> a new good experience both in technical discussion and personal life.
>
> No need to patronize me, especially not regarding my
> personal life.
Ok, so, you can do not pay attention to other people in your personal
life, but it would nice if you would try to understand other people here
on Harbour mailing list. We will have much more constructive discussions.
Regards,
Mindaugas