Path: news.eternal-september.org!eternal-september.org!feeder3.eternal-september.org!panix!.POSTED.spitfire.i.gajendra.net!not-for-mail From: cross@spitfire.i.gajendra.net (Dan Cross) Newsgroups: comp.os.vms Subject: Re: FreeTDS port to VMS V9.x on x86? Date: Thu, 3 Jul 2025 20:07:23 -0000 (UTC) Organization: PANIX Public Access Internet and UNIX, NYC Message-ID: <1046ntr$n6o$1@reader1.panix.com> References: <101n7gq$4m9b$1@dont-email.me> <68669c25$0$690$14726298@news.sunsite.dk> <1046fbb$44e$1@reader1.panix.com> <1046hc5$62th$2@dont-email.me> Injection-Date: Thu, 3 Jul 2025 20:07:23 -0000 (UTC) Injection-Info: reader1.panix.com; posting-host="spitfire.i.gajendra.net:166.84.136.80"; logging-data="23768"; mail-complaints-to="abuse@panix.com" X-Newsreader: trn 4.0-test77 (Sep 1, 2010) Originator: cross@spitfire.i.gajendra.net (Dan Cross) In article <1046hc5$62th$2@dont-email.me>, Arne Vajhøj wrote: >On 7/3/2025 1:40 PM, Dan Cross wrote: >> In article <68669c25$0$690$14726298@news.sunsite.dk>, >> Arne Vajhøj wrote: >>> On 7/1/2025 3:57 PM, Arne Vajhøj wrote: >>>> On 6/25/2025 8:15 AM, Craig A. Berry wrote: >>>>>                   feel free to take a crack at it and submit a PR.  To >>>>> work against current sources, you either need to use autoconf with a git >>>>> checkout or get a nightly snapshot from . >>>> >>>> I may take a look. >>>> >>>> Submitting a PR may be a problem. I have never had a personal >>>> Github account and I suspect that my work Github account has >>>> been deactivated. But I will worry about that when/if I have >>>> a good fix. >>> >>> First take: >>> >>> https://www.vajhoej.dk/arne/temp/tds_vasprintf.c > >>> Total rewrite. I did not like the old code at all. >> >> Your code is incorrect. >> >> Note that `vsnprintf` et al do not include space for the NUL >> terminator in their return value, and you do not account for >> this when you malloc your destination buffer. > >Ouch. Yeah, well, stupidly my own code had the same problem when I posted it, plus a bunch of other bogons, which is why my test did not catch the errors. Silly me. >I will add +1 everywhere for space. Then you should probably test for overflow, since *printf return a signed int, and signed integer overflow is UB. It is easier to do the arithmetic unsigned, as I have done. >Thanks. > >> 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. A helper function to abstract away some of the boilerplate is not going to hurt you. See below. >> You refer to `_PATH_DEVNULL` but do not `#include `, 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, `` 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. 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. Unfortunately, the C language does not provide any mechanism beyond the extremely crude notion of "translation units" and `static` for building namespaces that limit the visibility of non-local identifiers, and the preprocessor bypasses even that, so you have to resort to hacky and fragile things like manually adding text prefaces to have some approximation of the desired effect. But you have already got one of those, so you may as well use it. - Dan C. Here is a version that avoids nested #if: /* * SPDX-License-Identifier: LGPL 2.1 OR Apache 2.0 * * vasprintf implementation */ #include #include #include #include #include "config.h" static const size_t FAIL = ~(size_t)0; static inline int tds_vsnprintf(char *dst, size_t len, const char *fmt, va_list ap) { #ifdef HAVE_VSNPRINTF return vsnprintf(dst, len, fmt, ap); #else assert(dst != NULL); assert(len != 0); return vsprintf(dst, fmt, ap); #endif } #ifdef REENTRANT #define neednull(fp) 1 #define putnull(fp) fclose(fp) #else #define neednull(fp) ((fp) == NULL) #define putnull(fp) #endif static inline size_t tds_vasprintf_bufsize(const char *fmt, va_list ap) { int len; #if defined(HAVE_VSCPRINTF) len = _vscprintf(fmt, ap); #elif defined(HAVE_VSNPRINTF) len = vsnprintf(NULL, 0, fmt, ap); #else static FILE *fp = NULL; if (neednull(fp)) fp = fopen(TDS_PATH_DEVNULL, "w"); if (fp == NULL) return FAIL; len = vfprintf(fp, fmt, ap); putnull(fp); #endif if(len < 0) return FAIL; return (size_t)len + 1; } int tds_vasprintf(char **outp, const char *fmt, va_list ap) { #if defined(HAVE_VASPRINTF) return vasprintf(outp, fmt, ap); #elif !defined(va_copy) #error "va_copy macro required" #else va_list save_ap; va_copy(save_ap, ap); size_t len = tds_vasprintf_bufsize(fmt, ap); if (len == FAIL) return -1; char *dst = malloc(len); if (dst == NULL) return -1; tds_vsnprintf(dst, len, fmt, save_ap); *outp = dst; return (int)len; #endif } #ifdef TEST #include #include char * ========== REMAINDER OF ARTICLE TRUNCATED ==========