[GRASS5] segment lib speed patch

Hi,

to speed up the creation of segments in lib/segment/format.c,
a colleague from ITC-irst proposes the attached patch.
I used it locally for quite a while without having problems.

I would like to hear you opinion:
- any objections to include it (removing the old version)?
- should it be conditionalized for GNU/Linux only or
  anything else (means: modifying the current condition ORIG)?

Thanks

Markus

(attachments)

format.c.diff (822 Bytes)

+ G_warning("%s\n",strerror(errno));

Extraneous \n ?

H

Markus wrote:

+ return -1;

For a long time I have wondered what is best to use for return codes;
most modules return 0 for success as per UNIX norms, library fns return
all sorts of things, e.g. often 1 for true, etc.. glibc has the same
issues with e.g. strcmp(), and I understand is_foo() returning
TRUE/FALSE.

esp. for all the if(G_parser()) exit(-1); statements in the modules.
Should that really be -1 or just 1 or does it matter?

It would be nice if we had something standard we could put in the
SUBMITTING file. (e.g. action fn()s return 0 on success, test fn()s
return 1, or something like that?)

Specifically: should we avoid return -1; ??

slightly confused,
Hamish

On Tue, 2005-06-28 at 11:19 +1200, Hamish wrote:

Markus wrote:
> + return -1;

For a long time I have wondered what is best to use for return codes;
most modules return 0 for success as per UNIX norms, library fns return
all sorts of things, e.g. often 1 for true, etc.. glibc has the same
issues with e.g. strcmp(), and I understand is_foo() returning
TRUE/FALSE.

esp. for all the if(G_parser()) exit(-1); statements in the modules.
Should that really be -1 or just 1 or does it matter?

Isn't it best to use EXIT_FAILURE/EXIT_SUCCESS to avoid idiosyncrasies
between implementations?

It would be nice if we had something standard we could put in the
SUBMITTING file. (e.g. action fn()s return 0 on success, test fn()s
return 1, or something like that?)

Specifically: should we avoid return -1; ??

For normal functions that return int, I generally return -1 if some sort
of error function has been set, but return 0 otherwise. I don't know
how applicable that is to GRASS, off hand.

--
Brad Douglas <rez@touchofmadness.com>

Hi,

I got some useful offlist comments. Find attached the modified
patch along with the suggested HAVE_LSEEK condition.
The '\n' were old stuff, I have taken them out.

Please have a look again.

Markus

On Mon, Jun 27, 2005 at 02:04:40PM -0700, Brad Douglas wrote:

On Mon, 2005-06-27 at 17:03 +0200, Markus Neteler wrote:
> Hi,
>
> to speed up the creation of segments in lib/segment/format.c,
> a colleague from ITC-irst proposes the attached patch.
> I used it locally for quite a while without having problems.
>
> I would like to hear you opinion:
> - any objections to include it (removing the old version)?
> - should it be conditionalized for GNU/Linux only or
> anything else (means: modifying the current condition ORIG)?

It's a basic POSIX function, so you're safe. If you want to get
technical about it, then you could surround it with #ifdef HAVE_LSEEK...
generated by autoconf, which may arguably be better than #ifdef ORIG...

PS -
You don't need the '\n' in G_warning(), unless you intend on a double
carriage return.

--
Brad Douglas <rez@touchofmadness.com>

--
Markus Neteler <neteler itc it> http://mpa.itc.it
ITC-irst - Centro per la Ricerca Scientifica e Tecnologica
MPBA - Predictive Models for Biol. & Environ. Data Analysis
Via Sommarive, 18 - 38050 Povo (Trento), Italy

(attachments)

format.c.diff2 (1.87 KB)

On Mon, Jun 27, 2005 at 04:49:52PM -0700, Brad Douglas wrote:

On Tue, 2005-06-28 at 11:19 +1200, Hamish wrote:
> Markus wrote:
> > + return -1;
>
> For a long time I have wondered what is best to use for return codes;
> most modules return 0 for success as per UNIX norms, library fns return
> all sorts of things, e.g. often 1 for true, etc.. glibc has the same
> issues with e.g. strcmp(), and I understand is_foo() returning
> TRUE/FALSE.
>
> esp. for all the if(G_parser()) exit(-1); statements in the modules.
> Should that really be -1 or just 1 or does it matter?

Isn't it best to use EXIT_FAILURE/EXIT_SUCCESS to avoid idiosyncrasies
between implementations?

Yes, that's much more intuitive.
In DBMI we have DB_OK and and DB_FAILED.

> It would be nice if we had something standard we could put in the
> SUBMITTING file. (e.g. action fn()s return 0 on success, test fn()s
> return 1, or something like that?)
>
> Specifically: should we avoid return -1; ??

For normal functions that return int, I generally return -1 if some sort
of error function has been set, but return 0 otherwise. I don't know
how applicable that is to GRASS, off hand.

--
Brad Douglas <rez@touchofmadness.com>

Markus

Markus Neteler wrote:

to speed up the creation of segments in lib/segment/format.c,
a colleague from ITC-irst proposes the attached patch.
I used it locally for quite a while without having problems.

I would like to hear you opinion:
- any objections to include it (removing the old version)?
- should it be conditionalized for GNU/Linux only or
  anything else (means: modifying the current condition ORIG)?

It doesn't need to be conditionalised upon anything; it will work on
any Unix. On most Unices, you can simplify the code further by using
ftruncate() (although ftruncate() exists on all POSIX-compliant
systems, some older Unices don't allow you to use it to lengthen a
file).

The only potential objection is that the new version doesn't allocate
disk storage for the file; storage will be allocated dynamically as
blocks are actually written. This could result in zero_fill()
succeeding but a subsequent call to write() failing with ENOSPC ("No
space left on device").

Whether that's an advantage or disadvantage is a matter of preference.
If you're dealing with sparse data, the reduced storage is an
advantage. If you aren't, it's of no benefit, and it would usually be
preferable to fail during initialisation than at an intermediate
stage.

--
Glynn Clements <glynn@gclements.plus.com>

Hamish wrote:

> + return -1;

For a long time I have wondered what is best to use for return codes;
most modules return 0 for success as per UNIX norms, library fns return
all sorts of things, e.g. often 1 for true, etc.. glibc has the same
issues with e.g. strcmp(), and I understand is_foo() returning
TRUE/FALSE.

Predicates (pure functions which return truth values), e.g. the
isXXX() functions in ctype.h, normally return C truth values, i.e.
zero for false, non-zero for true.

Functions for which non-negative integers (include zero) are valid
results normally use -1 for errors.

Procedures which return a status typically use zero for success, and
-1 for errors, as that is how most system calls behave.

strcmp() returns -1, 0 or 1 for <, == or > respectively. Comparison
functions used by qsort() use the same convention.

esp. for all the if(G_parser()) exit(-1); statements in the modules.
Should that really be -1 or just 1 or does it matter?

It should be 1. Returning a negative value from main() is bogus. In
practice, it will typically be equivalent to returning 255 (the exit
code is usually 8 bits wide, although I'm not sure if this is mandated
by any formal standard).

It would be nice if we had something standard we could put in the
SUBMITTING file. (e.g. action fn()s return 0 on success, test fn()s
return 1, or something like that?)

Specifically: should we avoid return -1; ??

We should avoid returning -1 from main() and exit(-1).

It's a sensible return value for functions, particularly where all
non-negative integers are valid (non-error) results.

--
Glynn Clements <glynn@gclements.plus.com>