I am generally not enamored of non-standard string libraries. They add a new
dependency, they subject our code to the maintenance whims of other projects,
and there's an easy, standard fix that uses the standard C libraries.
For example, in the very code that we're referring to, there are both correct
and incorrect uses of strncat:
Correct:
strncat(comment,
" HAZMAT: Unknown Material",
comment_size - 1 - strlen(comment));
Incorrect:
strncat(comment, (char *)element, element_len);
In the first, we're calculating how much space is left in the comment array
based on the size of the string buffer and the current contents, making room for
a terminating '/0'. The size argument of strncat is intended to be the size
of the *available* space, and in the first case it is just that.
The second is incorrect, because it blindly all of the contents of "element"
into comment irrespective of how much space is left. The "size" argument
of the call is simply the number of things we want to copy in, not the
space available for copying it.
Fixing this may be a lot of typing, but it's not worth adding a whole new
layer of dependency and the sweeping code refactor that requires.
As I understand it, snprintf is not actually a standard C function, and
so it was necessary to provide a portable implementation in order for Xastir
to build universally. If snprintf *had* been a standard function,
xastir_snprintf would have been unnecessary and would be no better than the
system implemented snprintf.
strncat and friends are perfectly viable string manipulation functions in C
and I see no compelling reason to start chasing down alternate string
manipulation functions that would require large scale development.
There are other cases in Xastir where strncat was used incorrectly to copy,
say, the first three elements of a string into some other string, and it was
using the "incorrect" version of strncat to do it (with the size being the
number of characaters to copy rather than the maximum space available). In
*that* case, moving to a memcpy or a simple loop over array indices would be
the appropriate change.
In the case where we're tacking a string of arbitrary length onto an existing
string, strncat is the right tool for the job so long as it's called correctly.
On Sun, Dec 19, 2021 at 09:30:37AM -0800, we recorded a bogon-computron
collision of the <curt.w...@gmail.com> flavor, containing:
> Tom,
>
> Thanks for your recent explanation in the issue discussion (Issue
> #188). This is good general "string" talk and applicable to a wide
> range of code. For that reason I'm moving the discussion here so it's
> more easily found later, instead of hidden in Github issue comments.
>
> Pardon me while I summarize my limited knowledge of "strings" at this point:
>
> There aren't actually strings in C, only null-terminated character
> buffers. We can't run past the end or else we read/write from other
> stuff in memory.
>
> In Xastir we often use character buffers of size MAX_DEVICE_BUFFER,
> which is the case for the Opentrac code.
>
> At one point we got rid of almost all of the strnXX() type of calls,
> switching to xastir_snprintf() instead... Perhaps the Opentrac
> routines were coded up later or weren't converted at the time.
>
> >From Tom:
> -----
> "There was a lot if incorrect use of strncat in Xastir, mostly using
> incorrect values of "n" --- it's supposed to be the maximum number of
> characters that can fit into the destination string, and in many cases
> the code had been written to specify that number as the number of
> characters to copy out of the source string instead. We fixed a lot of
> that, but the opentrac code had a lot of it.
>
> The "right" fix is to fix the strncat calls so that one keeps track of
> how much room is left in the destination string, and use that as
> the"n" in strncat."
> -----
>
> I can certainly do that last but it seems like a lot of busy work in
> C. Other languages have string functions which handle all of this. Are
> there any C libraries that make it easier these days? I know I
> couldn't find a good one before and that's why xastir_snprintf() came
> about.
>
> --
> Curt, WE7U http://xastir.org http://www.sarguydigital.com
> _______________________________________________
> Xastir-dev mailing list
> Xastir-dev@lists.xastir.org
> http://xastir.org/mailman/listinfo/xastir-dev
--
Tom Russo KM5VY
Tijeras, NM
echo "prpv_a'rfg_cnf_har_cvcr" | sed -e 's/_/ /g' | tr [a-m][n-z] [n-z][a-m]
_______________________________________________
Xastir-dev mailing list
Xastir-dev@lists.xastir.org
http://xastir.org/mailman/listinfo/xastir-dev
