Bug 4910: “Folder option…” arreglado

 

El Bug 4910 ha sido arreglado por Gregor Schneider.

Cuando en ReactOS se intentaba abrir la ventana “Folder Options” del “Panel de Control” haciendo doble click sobre ella, se abrían dos ventanas en vez de una.(Un 2x1 en toda regla). En Windows solo se puede tener abierta una ventana de este tipo a la vez.

 

Además, como nota curiosa, GabrielIt descubrió que si se cliqueaba sobre “Folder Options” con el botón derecho y se elegía la opción “Abrir” del menu contextual, se abría una única ventana. Tal y como debería ser.

Gschneider se pusó manos a la obra y encontró el fallo.

 

El Bug

El problema residía en la comprobación a la que se sometía a la función ShellExecuteExW.

Esta función se encarga de realizar una operación en un fichero especificado(es una función bastante generalista), y devuelve TRUE si ha realizado la operación de manera satisfactoria o FALSE si ha ocurrido algun error.

Si ShellExecuteExW ha realizado la operación de manera satisfactoria, además, almacena en a hInstApp un valor mayor de 32. (Según MSDN)

En el código anterior(y con Bug) el checkeo se realizaba de la siguiente manera:

      ShellExecuteExW(&sei); 
        if (sei.hInstApp <= (HINSTANCE)32)

                             return E_FAIL;

 

Es decir, se comprobaba que el valor de hInstApp era menor a 32, lo que equivalía a Fallo, y se devolvía E_FAIL (indicando el fallo).

Para comprobar el fallo habría sido mucho mas sencillo comprobar el valor devuelto por ShellExecuteExW, y eso es lo que ha hecho Gregor

+      if (ShellExecuteExW(&sei) == FALSE)

                             return E_FAIL;

Es decir, comprobar directamente si falla o no ShellExecuteExW mirando lo que devuelve y no “el daño colateral”.

 

 

 

¿Pero por qué fallaba si ambas vías son correctas según MSDN?

 

La respuesta nos la da Gregor en el commit:

“Mejor no usar hInstApp pues puede ser un indicador no muy creible según http://msdn.microsoft.com/en-us/library/bb759784(v=VS.85).aspx “

Otra forma de resolver el Bug habría sido usar,

-       if (sei.hInstApp <= (HINSTANCE)32)

+       if ((INT)sei.hInstApp <= 32)

ya que hInstApp no es un verdadero HINSTANCE, tal y como dice el link anterior.

Un  problema menos ;)