Also, not that it has much weight: perror and strerror
are both C89 and C99 functions, and there are more non-standard
functions for handling errors. If a very mature language such as C
sees a need for so many, potentially duplicate, functions, I don't
see why Chicken Scheme isn't equally justified in having potentially
duplicate, but slightly different and still useful, functions.
Granted the above statement is very broad and general.
On 01/29/2015 04:55 AM, Alexej Magura
wrote:
Oh, and just for uhhh... completeness(?) the rationale is to make
debugging/working with low-level functions in Chicken scheme that
return errno easier, where printing out an error message may or
may not be desirable, as in the author has yet to decide if that
is best solution (even if that is the only sensible solution) and
just needs a function that'll quickly let him know what the errno,
a given function returns, means.
Also, I was not aware of posix-error prior to working on
this patch, and after a quick glance and trial I continued working
on the patch since posix-error requires more than just errno,
making it unsuitable for the immediate desired effect.
On 01/29/2015 01:06 AM, Peter Bex
wrote:
On Wed, Jan 28, 2015 at 04:59:13PM -0700, Alexej Magura wrote:
The patch attached adds a binding to /strerror /to
posix-common.scm^* (version 4.9.0.1). The other attachment is the
patched file, to make it easier to merge the changes with upstream.
I've already tested the changes: it compiles and works, at least in
/csi/.
^* /strerror/ is specified by both C89 and C99, so I figure that
it's probably available on both *nix and Windows systems, hence
adding it in posix-common.scm instead of posixunix.scm or some other
file.
Hello Alexej,
First off, thanks for your effort. We always appreciate contributions
from new people!
What I'm missing from your mail is a motivation as to why this needs
to be in CHICKEN core; it's trivially added to user code, and the cases
in which this is useful are (AFAIK) rather limited: As far as I can tell,
this would be useful only if you're calling some other C functions
directly in your own code and extracting errno. Besides, there's
already "posix-error" which does much the same (but also signals an
error condition, which is usually what you'd want if you're already
extracting the human-readable error string).
Furthermore a few more comments about the patch itself:
- Your patch seems to modify several lines with no change(?). Perhaps
this is an automatic whitespace cleanup from your editor or something,
but that means the patch isn't exactly "clean": the ideal patch only
changes what's necessary (and perhaps cleans up only immediately
surrounding code). This makes it easier to apply to different
branches or after several changes have been made, for example.
- Sending the complete new file is unnecessary. We can't use it except
for generating the same patch, anyway.
- It's better to send the output of "git format-patch"; this will
automatically credit the commit to you in the git history, and
allows you to determine the best commit message to accompany this
change in the history.
- You didn't add this new procedure to the manual.
- You didn't add the new procedure to posix.import.scm, so it wont't
be available to code that's in a module.
Please don't let my comments discourage you from sending more patches,
I'm giving this feedback to help you send better patches in the future!
Cheers,
Peter
|