Partager via

Checking file versions is surprisingly hard.

  • Article

I was wandering around the web the other day and ran into this post.  In general I don’t have many issues with the post, until you get to the bottom of the article.  The author mentions that his code only runs on Win7 or newer so he helpfully included a check to make sure that his code only runs on WIn7:

 // Example in C#.

internal bool SupportsTaskProgress() {
    if (System.Environment.OSVersion.Version.Major >= 6) {
        if (System.Environment.OSVersion.Version.Minor >= 1) {
            return true;
        }
    }
    return false;
}

This is a great example of why it’s so hard to write code that checks for versions.  The problem here is that this code is highly likely to fail to work on the next version of Windows (or whenever Windows 7.0 is released).  In that case SupportsTaskProgress will incorrectly return false.

 

Personally I wouldn’t even bother writing the SupportsTaskProgress function this way.  Instead I’d check for the “new TaskbarLib.TaskbarList()” call to return NULL and assume that if it returned NULL the API call wasn’t supported (the non COM interop equivalent would be to check for a failure on the call to CoCreateInstance).  That way the code would work even if (for some obscure reason) the taskbar logic was ported to a previous OS version.

 

If I simply HAD to keep the SupportsTaskProgress function, I’d rewrite it as:

 // Example in C#.

internal bool SupportsTaskProgress() {
    if (System.Environment.OSVersion.Version.Major >= 6) {
        if (System.Environment.OSVersion.Version.Major == 6) {
            if (System.Environment.OSVersion.Version.Minor >= 1) {
                return true;
            }
            return false;
        }
        return true;
    }
    return false;
}

That way it would only check for minor version being greater than 1 if the major version is 6.  I suspect that this code could be tightened up further as well.

 

This is a part of the reason that picking a version number for the OS is so complicated.

Comments

  • Anonymous
    March 05, 2009
    OK, I've seen people code like this and have never figured out why. Given that the && operator will stop evaluating as soon as one condition returns false, why not use that to combine the IF statements?

  • Anonymous
    March 05, 2009
    SlackMaster K: I wanted to replicate the original style.  See my "I suspect that this code could be tightened up further as well" comment.

  • Anonymous
    March 05, 2009
    You could do: internal bool SupportsTaskProgress() {   return System.Environment.OSVersion.Version >= new Version(6, 1) } That should be equivalent.

  • Anonymous
    March 05, 2009
    In the general case for Versions V, you have M.m.r.p (Major, minor, revision and patch—or whatever they're called).  To compare two, you have to start with the major one and cascade: if (M1 > M2 ||     (M1 == M2 &&       (m1 > m2 ||         (m1 == m2 &&           (r1 > r2 ||             (r1 == r2 &&                (p1 > p2))))))) {    // V1 > v2 } I've not tried to optimize it beyond that, as it doesn't need to happen that often.

  • Anonymous
    March 05, 2009
    Hey all - I'm the "author" mentioned above. I won't go into the reasons of the brain-fart with the code as the original post that contains it has been updated to explain why, but one thing I'd like to mention is (which is directly above the actual code in the original posting): "The comparisons haven’t been short-circuited due to reasons of clarity." The visitors of my site tend to be end users as opposed to programmers (although some are beginners); I've learnt in the past to keep code examples very simple otherwise I get "emails". I've now updated the code and also written it so it's less readable for the visitors to my site. cough

  • Anonymous
    March 05, 2009
    We shouldn't blame the task as being hard.  The concept here is easy.  It's that the design of the API makes it hard.  So someone should fix the design and add a Win32 function that checks the version like Paul's code shows. In general, APIs should make easy things easy and complex things hard, not the other way around.

  • Anonymous
    March 05, 2009
    BootBlock: Read your email please.

  • Anonymous
    March 05, 2009
    Slackmaster: Not exactly like this, but I've broken up conditionals like this just to make debugging easier. As I'm stepping through something particularly complicated in a debugger, it's nice to know why I'm in (or not in) this code without having to count parens, consider precedence, and deal with the odd assignment/ternary embedded in an overworked conditional all the while doing whatever it takes to get the values of the variables IN the statement.

  • Anonymous
    March 05, 2009
    Like Paul says. This is trivial in .net since Version implements a greater than operator and icomparable

  • Anonymous
    March 05, 2009
    Let .net do the "heavy lifting" for you: Version win7 = new Version("7.1"); Version osVersion = new Version(System.Environment.OSVersion.Version.Major, System.Environment.OSVersion.Version.Minor) if(osVersion >= win7) ...

  • Anonymous
    March 05, 2009
    The comment has been removed

  • Anonymous
    March 06, 2009
    If I was doing it in C I'd be tempted to put the major version in the top bits of an integer and the minor version in the bottom bits and compare. E.g. #define PACK_VERSION(major, minor) ((major<<16)|minor) Then you can just compare integers. That said it feels wrong to do it this way rather than checking which functions are available. Actually, I'd rather just code to a subset of Win32 that has been around for ages because I've got less chance of breaking stuff on one OS.

  • Anonymous
    March 06, 2009
    The comment has been removed

  • Anonymous
    March 06, 2009
    Anon, the idea of packing major and minor into an int actually doesn't work that well - we tried it in DOS years and years ago. I actually love the .Net solution (make Version an icomparable and then move the logic somewhere lower).  The cool thing is that it's essentially available in unmanaged code via theVerifyVersionInfo API: http://msdn.microsoft.com/en-us/library/ms725492(VS.85).aspx But it doesn't change my base comment: Don't ever check windows versions.  Instead check for functionality being present or not.  

  • Anonymous
    March 06, 2009
    I somehow find it ironical that someone has trouble with comparing versions and can get COM right.. :|

  • Anonymous
    March 06, 2009
    The comment has been removed

  • Anonymous
    March 06, 2009
    The comment has been removed

  • Anonymous
    March 06, 2009
    Koro: Why not use LoadLibrary/GetProcAddress on OpenThemeData and then have a boolean check?

  • Anonymous
    March 06, 2009
    On my previous post , Koro made the following comment : “Don't ever check windows versions.&#160; Instead

  • Anonymous
    March 06, 2009
    Perhaps I missed something, but shouldn't the check be... System.Environment.OSVersion.Version >= New Version(6, 1)

  • Anonymous
    March 06, 2009
    The comment has been removed

  • Anonymous
    March 06, 2009
    The comment has been removed

  • Anonymous
    March 08, 2009
    What I want to know is, why is the format of GetVersion() so stupid? 9x does not have build number, NT has build number, but you have to mask it off because the minor ver is also packed there even tho it has already been included in the other WORD (IIRC)

  • Anonymous
    March 08, 2009
    > Instead I’d check for the “new TaskbarLib.TaskbarList()” call to return NULL and assume that if it returned NULL the API call wasn’t supported (the non COM interop equivalent would be to check for a failure on the call to CoCreateInstance). Wait, would it actually return null or would it throw an exception?  (I haven't really used COM from .NET, so I'm curious.)  The former would be a large surprise (it's not really something that "new" is supposed to do), and the latter has negative performance consequences.  (Then again, you probably shouldn't be allocating COM objects anywhere that perf matters anyway.)

  • Anonymous
    March 08, 2009
    Oh, and yeah, doing cascading compares like this in a readable fashion is really easy (as Dave showed above): static int Compare(MyType one, MyType two) {  if (one.field1 < two.field1) { return -1; }  if (one.field1 > two.field1) { return 1; }  // repeat the above two lines for each field, in order of decreasing significance  return 0; } It's really surprising how many people get this wrong.  It's even more surprising when someone gets this wrong even when the type being compared manually already has comparison operators, as Paul pointed out.

  • Anonymous
    March 12, 2009
    BTW, if you use VerifyVersionInfo on Server 2003 RTM to check for XP SP2, does it return TRUE or FALSE?

  • Anonymous
    April 09, 2009
    Here is a blog article by Raymond Chen on this: http://blogs.msdn.com/oldnewthing/archive/2004/02/13/72476.aspx

  • Anonymous
    April 18, 2009
    "That way the code would work even if (for some obscure reason) the taskbar logic was ported to a previous OS version." I already mentioned in a comment that appears to be deleted how HeapSet/QueryInformation was backported to Win2000 in a hotfix. In this month's security update, SetSearchPathMode was backported to Vista and XP, though not Win2000. What is sad was that the Windows SDK docs did not document these changes very well. For example, HeapSetInformation says that it was in Win2000 SP4, while it really ended up in a post-SP4 hotfix (actually it ended up in a time window that resulted in two versions, the older being only compatible with SP3, and the newer being compatible with SP4 as well), while HeapQueryInformation don't say that it is compatible with Win2000 as well. I hope MS will do better with documenting SetSearchPathMode.