• (nodebb)

    they don't really think

    This is TRWTF. The addendum "too much" after "think" is of no importance, and in any event isn't correct. It should be dropped in favour of "enough" or even "at all". And that's bad, because building software is a task for people who think about what they are doing.

  • Vera (unregistered)

    So, two WTFs for the price of one. On the one hand, "every version is 1.0!" On the other hand, "Yeah, we'll just check if one of the strings has ran out, not like file names will get longer/shorter between versions."

    I just hope that the latest "1.0" version (minus the old backup) doesn't introduce a boatload of new issues, it looks like they have plenty already.

  • (nodebb)

    every version is 1.0!

    For when your company's software is so well-written that you decide to create your own problems.

  • (nodebb) in reply to Vera

    every version is 1.0!

    Makes me wonder what actually gets updated between software "versions"

  • Foo AKA Fooo (unregistered)

    Also, correct me if I'm wrong, but it seems like a malicious set of filenames could cause buffer overruns here.

    If so, this would be serious, but I don't see it. I suppose you're referring to the lack of an explicit "*p2 != '\0'" check, but "*p2 >= '0'" (note: digit '0', which is different from and greater than NUL) also serves this purpose. So if p2 is at the end of the string, it will either (if p1 is too) reach the final return (which is fine, reading the NUL at the end of the strings), or (if p1 is a digit) reach the "return 1", otherwise "*p1 != *p2" will be true and it will reach the second-last return. It might not be the clearest control flow, but I don't see any case where p2 is incremented beyond the terminating NUL. Am I missing something?

  • Wyatt (unregistered)

    If only there were some sort of version structure embedded in the file contents so you didn't have parse a filename.... Oh wait, that's a different operating system.

  • (nodebb)

    It occurs to me that this line :

      return *p1 - *p2;

    might have unspecified (not undefined) behaviour, for example if you run it on a machine where char is unsigned and, simultaneously, char, short, and int are all the same size, because the difference is calculated (regardless of anything else) as unsigned on such a machine if the compiler uses the equivalent of -funsigned-char, as there are values of char in such a machine that cannot be represented by an int. (Ref: https://cppreference.com/w/c/language/conversion.html#Integer_promotions) What happens next is "unspecified" rather than "undefined" because the language standards say so, but might well cause you some ... fun... (see e.g. https://cppreference.com/w/c/language/conversion.html section "Integer conversions", specifically the last bullet point, referring to ):

    . otherwise, if the target type is signed, the behavior is implementation-defined (which may include raising a signal).

    Fortunately for us, there aren't many machines that have bitsof(char) == bitsof(int)

    Addendum 2026-02-10 15:01: "referring to )" should, of course, be "referring to unsigned to signed conversions at the same size, or any physically narrowing conversion)"

  • Tim R (unregistered)

    Honestly just looking at this code makes me glad that most of us don't have to use C nowadays. I propbably could understand it if I wanted but actually I think even a regex would be easier than this (and string.split would probably be the most readable solution).

  • (nodebb) in reply to Tim R

    Are you seriously proposing to use a regex to see which of two strings is greater(1)?

    (1) With a hybrid non-numeric-bits-in-asciibetic-order / numeric-bits-in-numeric-order comparison, no less.

  • (nodebb) in reply to Wyatt

    And you could even call it a VERSIONINFO resource... ;)

  • (nodebb)

    Dammit, it's got buffer overruns, integer overflows, and unchecked pointers, but no null pointer derefs that I can detect. They'll have to try harder next time.

  • (nodebb) in reply to zomgwtf

    Dammit, it's got buffer overruns, integer overflows, and unchecked pointers, but no null pointer derefs that I can detect. They'll have to try harder next time.

    There's no buffer overrun here unless the caller has already overrun the buffer. (This function does not modify the strings in any way.)

    There's definitely a possibility of integer overflows, if one or other or both of the two strings contained something like 5000000000 (five milliard, which is a bit more than the 4.3-ish milliard capacity of an int on I32 architecture. Heck, even 2.2 milliard would cause signed-int overflow which is UB. But then again, how likely are we to encounter version 1.2.2200000000 of a shared object?

    The pointers are, indeed, unchecked, which leads me to say that we do, indeed, have two potential null-derefs, at the first use of *p1 and the first use of *p2.

  • (nodebb) in reply to Steve_The_Cynic

    I meant that it'll do a read-overrun of the buffer into unallocated (or at least unknown) space, presumably triggering a segfault or other protection violation.

  • (nodebb) in reply to zomgwtf

    How do you reason that?

    My analysis says that it stops when it reaches the end of string "p1", of necessity given the outermost loop, and it stops analysing numbers when the relevant string contains a non-digit (and '\000', no matter if you use one, two, or three zeros, is a non-digit).

    So how can it escape from one or other of the strings?

  • (nodebb) in reply to zomgwtf

    it'll do a read-overrun

    If we are inside the loop, *p1 is not '\0', If*p1 is not an ASCII numeric and *p2 is '\0' the following branch will execute and leave the loop

        else if (*p1 != *p2)
      return *p1 - *p2;

    If*p1 is an ASCII numeric and *p2 is '\0' then the else branch of this conditional will be taken

      if (*p2 >= '0' && *p2 <= '9') {

        /* Snip the comparison nonsense  */

      } else
        return 1;

    All paths through the loop where *p2 is '\0' end in a return thus exiting the loop and the function.

    Addendum 2026-02-12 07:00: In the "comparison nonsense, p2 is advanced but it starts out non nul and the while never advances it beyond the nul. The worst case scenario is it is pointing at a nut at the end of the inner loop and the next iteration of the outer loop will take care of it safely.

    Addendum 2026-02-12 07:01: Sorry about all the typos in the addendum

Leave a comment on “One Version of Events”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article