Re: FreeTDS port to VMS V9.x on x86?

In article <10476qu$edc3$1@dont-email.me>,
Arne Vajhøj  <arne@vajhoej.dk> wrote:

On 7/3/2025 4:07 PM, Dan Cross wrote:

In article <1046hc5$62th$2@dont-email.me>,
Arne Vajhøj  <arne@vajhoej.dk> wrote:

On 7/3/2025 1:40 PM, Dan Cross wrote:

As the main logic is the same in each case (find out the buffer
size, allocate, and then assign and return), and the only real
difference is in finding out the buffer length, your code would
be more readable with some helper functions that you delegated
to.

>
As states in the comments then it was a design goal to have
simple code blocks - no nested if's, no functions - for a given
context just 1-10 lines of code.

 
That's a bad design goal.
 
This sort of organization leads to lots of duplication and tends
to produce code that is overly verbose and fragile, where one
section may be updated and the others skipped or missed.  A
little bit of duplication for the sake of comprehensibility is
ok, but this is excessive.

>
It is very easy to read this way.
>
Just scan sequentially down until one find the relevant
#if context and then read 1-10 lines sequentially.
No jumping up and down.

Well, it's subjective whether it's readable or not.

But I didn't say that each stanza was difficult to read.  I said
that the repetition makes the code overly verbose (because so
much boilerplate is duplicated) and fragile (because it's easy
to miss a place that needs to be updated because of all the
repetition).

Those critics remain true.

You refer to `_PATH_DEVNULL` but do not `#include <paths.h>`, as
required by POSIX.

>
This must support non *nix systems (as an example
VMS !!) - config.h is expected to provide that with the rest.

 
If copyright dates are anything to judge by, `<paths.h>` has
been a thing since 1989, but I was wrong in that it is not
actually in POSIX: it is a BSD extension, though common.

>
This code should also work on non-*nix.

Yup.  That's why you shouldn't use `_PATH_DEVNULL` in the first
place.

It does not matter if all or most *nix has had it since 1989.

I just think it's sad that the header hasn't been picked up by
other systems in 35 years, but I think you missed the larger
overall point.

In general, it is good practice and good hygiene in C programs
to `#include` the header files that are documented to define the
symbols and types that you use in your program, instead of
relying on transitive includes to populate things ambiantly via
e.g. `config.h`.
 
Regardless, in that case, you shouldn't use `_PATH_DEVNULL`.
Note the leading `_`: that signifies a reserved identifier,
which is not something you should be defining yourself.  If you
want to punt this to e.g. config.h, better would be to define a
new name (say, `TDS_PATH_DEVNULL`) and use that.

>
This has to fit into an existing project that has its
way to setup things.

Let's be clear: this entire interface is pretty trivial.  There
are no great mysteries here, and munging this code to be a bit
cleaner won't have a major impact on the rest of the project.
Arguments about "fitting into the existing project" are thus
not terribly persuasive for this specific example.

Note that the code that I posted made liberal use of `static` to
hide helper functions and so on; moreover, there is no great
burden added by using some new identifier that is not
`_PATH_DEVNULL` (which is non-portable anyway).

I also think that its way makes sense. It would not be good
to start putting OS specific includes/defines into lots
of C files. That is what the configure process is supposed to
handle.

I don't know why you brought this up; I don't think anyone
suggested putting OS-specific things "into lots of C files."

Actually, the OS-specific identifier is `_PATH_DEVNULL`, which
is already in use.  Since it became evident that the header file
it comes from is not portable, my suggestion is to _not_ use
that identifier, but instead introduce a new identifier that can
be defined in a single place using OS-specific code (e.g., some
kind of compatability header) and use that throughout.  That
may be defined in terms of `_PATH_DEVNULL` on systems where
<paths.h> exists, but critically that would be isolated in a
single place: no need to sprinkle OS-specific code all over the
place.

Re: header files, code files that make use of an identifier
should `#include` the header file that identifier is documented
to come from (where "documentation" is some relevant standard,
a man page, or other system- or library-specific documentation).
This generally makes builds more efficient, aids tools for
static analysis, IDEs, linters, makes unit-level testing easier,
and highlights problematic design areas (see below).

You should _not_ have some "god" header that #include's a bunch
of stuff, and is in turn included everywhere.  This leads to
presenting bloat to the compiler, increasing build times and
wasting resources, polluting the global namespace unnecessarily,
and leads to hacks going into headers that are a drag on
maintenace going forward.

Indeed, <paths.h> and using `_PATH_DEVNULL` is a good example of
the downsides here: since one cannot portably include the header
where it is most commonly defined in the places where that
symbol is used, this strongly suggests that the symbol is being
used in places where it shouldn't.  This is an example of a
non-portable, system-specific symbol being shoe-horned into use
on a system where it's not otherwise defined.

If you need to depend on specific _features_, then define
macros for those (as autoconf does).  Using system definitions
as a proxy for what features exist is poor practice.  Fun fact:
I was the person who put that policy in place at Google, from
which it became the mechanism for configuring options in Abseil
(e.g., https://abseil.io/docs/cpp/platforms/feature_checks).

- Dan C.