Alguien me sugirió recientemente que revisara las bibliotecas de Visual Studio 2013. No encontré nada de mucho interés, solo algunos pequeños errores y deslices. No serían un artículo interesante y atractivo, pero aun así he decidido describir todos esos defectos. Solo espero que ayude a mejorar un poco las bibliotecas y estimule a los autores a realizar un análisis más completo. No tengo los archivos de proyecto necesarios para construir las bibliotecas, por lo que mi análisis tuvo que ser superficial y podría haberme perdido mucho.
Este es el segundo artículo sobre el análisis de las bibliotecas de Visual C++. Para ver los resultados de la comprobación anterior, consulte el artículo Errores detectados en las bibliotecas de Visual C++ 2012.
No puedo analizar las bibliotecas en su totalidad y lo que hice fue una verificación bastante descuidada:incluí en un nuevo proyecto todos los archivos de las carpetas "crt\src" y "atlmfc\src" y también creé un nuevo archivo test.cpp para incluir todos los archivos de encabezado relacionados con la biblioteca estándar (vector, mapa, conjunto, etc.) en.
Después de eso, jugué un poco con la configuración del proyecto y finalmente logré compilar alrededor del 80% de los archivos. Creo que eso es suficiente. Incluso si un archivo no puede compilarse, PVS-Studio generalmente puede verificarlo de todos modos, aunque solo sea parcialmente.
Creo que si los desarrolladores de bibliotecas encuentran útil e interesante este artículo, realizarán un análisis más exhaustivo. Incluso el proceso de construcción exótico ya no es un problema, ya que puede usar el sistema de monitoreo del compilador.
Usé PVS-Studio 5.19 para hacer el análisis. Revisé los códigos fuente de las bibliotecas C/C++ incluidas en Visual Studio 2013 (actualización 3).
Resultados del análisis
También encontré algunos defectos que se encontraron en la versión anterior, Visual Studio 2012. Por ejemplo, la función proj() todavía se implementa de una manera bastante extraña; el destructor ~single_link_registry() está escrito de la misma manera peligrosa. Pero no es interesante contar la misma historia. Tratemos de encontrar algo nuevo.
Comprobación de índice incorrecta
void _Initialize_order_node(...., size_t _Index, ....)
{
if (_Index < 0)
{
throw std::invalid_argument("_Index");
}
....
}Mensaje de diagnóstico de PVS-Studio:V547 La expresión '_Index <0' siempre es falsa. El valor de tipo sin firmar nunca es <0. agent.h 8442
El argumento '_Index' no está firmado. Es por eso que la verificación no tiene ningún sentido porque nunca se generará una excepción. Parece código superfluo en lugar de un error.
Formato incorrecto
int _tfpecode; /* float point exception code */
void __cdecl _print_tiddata1 (
_ptiddata ptd
)
{
....
printf("\t_gmtimebuf = %p\n", ptd->_gmtimebuf);
printf("\t_initaddr = %p\n", ptd->_initaddr);
printf("\t_initarg = %p\n", ptd->_initarg);
printf("\t_pxcptacttab = %p\n", ptd->_pxcptacttab);
printf("\t_tpxcptinfoptrs = %p\n", ptd->_tpxcptinfoptrs);
printf("\t_tfpecode = %p\n\n", ptd->_tfpecode);
....
}Mensaje de diagnóstico de PVS-Studio:V576 Formato incorrecto. Considere verificar el segundo argumento real de la función 'printf'. El puntero se espera como argumento. tidprint.c 133
Lo que estamos tratando aquí es el efecto de la última línea. Hay un error al final de un bloque de líneas de aspecto similar. En cada línea, se debe imprimir un valor de puntero, pero en la última línea, la variable '_tfpecode' es solo un valor entero, no un puntero. Lo que debería haberse escrito en su lugar es lo siguiente:
printf("\t_tfpecode = %i\n\n", ptd->_tfpecode);Extraños cálculos repetidos
unsigned int SchedulerProxy::AdjustAllocationIncrease(....) const
{
....
unsigned int remainingConcurrency =
m_maxConcurrency - m_currentConcurrency;
remainingConcurrency = m_maxConcurrency - m_currentConcurrency;
....
}Mensaje de diagnóstico de PVS-Studio:V519 A la variable 'remainingConcurrency' se le asignan valores dos veces seguidas. Quizás esto sea un error. Comprobar líneas:1136, 1137. schedulerproxy.cpp 1137
A la variable se le asigna dos veces el resultado de una misma expresión. Este código es superfluo y lo más probable es que sea el resultado de una refactorización deficiente.
Sospecha de error tipográfico
double HillClimbing::CalculateThroughputSlope(....)
{
....
MeasuredHistory * lastHistory = GetHistory(fromSetting);
MeasuredHistory * currentHistory = GetHistory(toSetting);
....
double varianceOfcurrentHistory = currentHistory->VarianceMean();
double varianceOflastHistory = currentHistory->VarianceMean();
....
}Mensaje de diagnóstico de PVS-Studio:V656 Las variables 'varianceOfcurrentHistory', 'varianceOfflastHistory' se inicializan mediante la llamada a la misma función. Probablemente sea un error o un código no optimizado. Considere inspeccionar la expresión 'currentHistory->VarianceMean()'. Consultar líneas:412, 413. hillclimbing.cpp 413
Es sospechoso que a las variables varianceOfcurrentHistory y varianceOfflastHistory se les asigne el mismo valor. Sería más lógico inicializar la variable varianceOfflastHistory de la siguiente manera:
double varianceOflastHistory = varianceOfcurrentHistory;Además, también está el puntero 'lastHistory'. Mi suposición es que hay un error tipográfico en este código y lo más probable es que se viera así:
double varianceOfcurrentHistory = currentHistory->VarianceMean();
double varianceOflastHistory = lastHistory->VarianceMean();Error tipográfico genuino
BOOL CPropertySheet::SetActivePage(CPropertyPage* pPage)
{
ASSERT_VALID(this);
ENSURE_VALID(pPage);
ASSERT_KINDOF(CPropertyPage, pPage);
int nPage = GetPageIndex(pPage);
ASSERT(pPage >= 0);
return SetActivePage(nPage);
}Mensaje de diagnóstico de PVS-Studio:V503 Esta es una comparación sin sentido:puntero>=0. dlgprop.cpp 1206
Es extraño verificar que el valor de un puntero sea mayor o igual a cero. Obviamente, esto es un error tipográfico y el programador en realidad quería verificar la variable 'nPage':
int nPage = GetPageIndex(pPage);
ASSERT(nPage >= 0);Eso es solo un ASSERT por supuesto y el error no causará ningún problema serio, pero sigue siendo un error.
Ejecutar las mismas acciones independientemente de la condición
void CMFCVisualManager::OnDrawTasksGroupCaption(....)
{
....
if (pGroup->m_bIsSpecial)
{
if (!pGroup->m_bIsCollapsed)
{
CMenuImages::Draw(pDC, CMenuImages::IdArrowUp,
rectButton.TopLeft());
}
else
{
CMenuImages::Draw(pDC, CMenuImages::IdArrowDown,
rectButton.TopLeft());
}
}
else
{
if (!pGroup->m_bIsCollapsed)
{
CMenuImages::Draw(pDC, CMenuImages::IdArrowUp,
rectButton.TopLeft());
}
else
{
CMenuImages::Draw(pDC, CMenuImages::IdArrowDown,
rectButton.TopLeft());
}
}
....
}Mensaje de diagnóstico de PVS-Studio:V523 La declaración 'entonces' es equivalente a la declaración 'else'. afxvisualmanager.cpp 2118
Independientemente de la condición (pGroup->m_bIsSpecial), se ejecutan las mismas acciones. Eso es extraño.
Comprobación de número de puerto incorrecto
typedef WORD ATL_URL_PORT;
ATL_URL_PORT m_nPortNumber;
inline BOOL Parse(_In_z_ LPCTSTR lpszUrl)
{
....
m_nPortNumber = (ATL_URL_PORT) _ttoi(tmpBuf);
if (m_nPortNumber < 0)
goto error;
....
}Mensaje de diagnóstico de PVS-Studio:V547 La expresión 'm_nPortNumber <0' siempre es falsa. El valor de tipo sin firmar nunca es <0. atlutil.h 2773
La variable 'm_nPortNumber' tiene el tipo WORD sin firmar.
Falta destructor virtual
class CDataSourceControl
{
....
~CDataSourceControl();
....
virtual IUnknown* GetCursor();
virtual void BindProp(....);
virtual void BindProp(....);
....
}
CDataSourceControl* m_pDataSourceControl;
COleControlSite::~COleControlSite()
{
....
delete m_pDataSourceControl;
....
}Mensaje de diagnóstico de PVS-Studio:V599 El destructor no se declaró como virtual, aunque la clase 'CDataSourceControl' contiene funciones virtuales. occsite.cpp 77
La clase CDataSourceControl contiene métodos virtuales pero el destructor no es virtual. Eso es peligroso:si una clase X se hereda de la clase CDataSourceControl, no podrá destruir objetos del tipo X utilizando un puntero a la clase base.
Código incompleto
BOOL CMFCWindowsManagerDialog::OnHelpInfo(HELPINFO* pHelpInfo)
{
pHelpInfo->iCtrlId;
CWnd* pParentFrame = AfxGetMainWnd();
pParentFrame->SendMessage(AFX_WM_WINDOW_HELP, 0,
(LPARAM) this);
return FALSE;
}Mensaje de diagnóstico de PVS-Studio:V607 Expresión sin propietario 'pHelpInfo->iCtrlId'. afxwindowsmanagerdialog.cpp 472
¿Qué es "pHelpInfo->iCtrlId;"? ¿Qué significa?
Doble inicialización sospechosa
CMFCStatusBar::CMFCStatusBar()
{
m_hFont = NULL;
// setup correct margins
m_cxRightBorder = m_cxDefaultGap; // <=
m_cxSizeBox = 0;
m_cxLeftBorder = 4;
m_cyTopBorder = 2;
m_cyBottomBorder = 0;
m_cxRightBorder = 0; // <=
....
}Mensaje de diagnóstico de PVS-Studio:V519 A la variable 'm_cxRightBorder' se le asignan valores dos veces seguidas. Quizás esto sea un error. Comprobar líneas:74, 80. afxstatusbar.cpp 80
Al principio, se escribe un valor de otra variable en la variable 'm_cxRightBorder'. Y luego, de repente, se pone a cero.
Comprobación de estado sospechoso
#define S_OK ((HRESULT)0L)
#define E_NOINTERFACE _HRESULT_TYPEDEF_(0x80004002L)
HRESULT GetDocument(IHTMLDocument2** ppDoc) const
{
const T* pT = static_cast<const T*>(this);
return pT->GetDHtmlDocument(ppDoc) ? S_OK : E_NOINTERFACE;
}
HRESULT GetEvent(IHTMLEventObj **ppEventObj) const
{
....
if (GetDocument(&sphtmlDoc))
....
}Mensaje de diagnóstico de PVS-Studio:V545 Dicha expresión condicional del operador 'if' es incorrecta para el valor de tipo HRESULT 'GetDocument(&sphtmlDoc)'. En su lugar, se debe utilizar la macro SUCCEEDED o FAILED. afxhtml.h 593
El formato del código no parece cumplir con la lógica de ejecución del código. Lo que puede pensar al principio es que si la condición 'GetDocument(...)' es verdadera, ha logrado obtener el documento. Pero en realidad es todo muy diferente. La función GetDocument() devuelve un valor del tipo HRESULT. Y todo es diferente en este tipo. Por ejemplo, el estado S_OK se codifica como 0 y el estado E_NOINTERFACE como 0x80004002L. Para verificar valores del tipo HRESULT, se deben usar macros especiales:SUCCEEDED, FAILED.
No sé con certeza si hay un error aquí, pero aún así este código es confuso y necesita ser revisado.
Argumento incorrecto para la macro MAKE_HRESULT
#define MAKE_HRESULT(sev,fac,code) \
((HRESULT) \
(((unsigned long)(sev)<<31) | \
((unsigned long)(fac)<<16) | \
((unsigned long)(code))) )
ATLINLINE ATLAPI AtlSetErrorInfo(....)
{
....
hRes = MAKE_HRESULT(3, FACILITY_ITF, nID);
....
}Mensaje de diagnóstico de PVS-Studio:V673 La expresión '(unsigned long)(3) <<31' se evalúa como 6442450944. Se requieren 33 bits para almacenar el valor, pero la expresión se evalúa como el tipo 'unsigned' que solo puede contener '32 ' bits. atlcom.h 6650
El código funcionará como debería, pero todavía hay un error en su interior. Siga leyendo para conocer la explicación.
La función debe formar un mensaje de error dentro de una variable del tipo HRESULT. Para ello, se utiliza la macro MAKE_HRESULT. Sin embargo, se usa incorrectamente. El programador asumió que el primer parámetro 'gravedad' toma valores dentro del rango entre 0 y 3. Debe haberlo confundido con la forma en que se forman los códigos de error cuando se trabaja con las funciones GetLastError()/SetLastError().
La macro MAKE_HRESULT solo puede tomar 0 (éxito) o 1 (fallo) como primer argumento. Para obtener detalles sobre este tema, consulte el foro en el sitio de CodeGuru:¡Advertencia! La macro MAKE_HRESULT no funciona.
Dado que el número 3 se utiliza como primer argumento real, se produce un desbordamiento. El número 3 se "convertirá" en 1. Este afortunado accidente evita que el error afecte la ejecución del programa.
ASSERT con condiciones siempre verdaderas
Hay bastantes fragmentos en los que se implementa una condición ASSERT en el patrón (X>=0). Al mismo tiempo, una variable X se declara como el tipo entero sin signo. Entonces la condición resulta ser siempre cierta.
En algunos casos, el uso de ASSERT como ese es válido, p. cuando una variable puede firmarse debido a la refactorización y el algoritmo no está listo para manejar números negativos. En este código, sin embargo, usar algunos de esos no parece tener ningún sentido. Deben eliminarse del código o reemplazarse con otras comprobaciones útiles. Por eso decidí mencionarlos en el artículo.
Mira este ejemplo:
DWORD m_oversubscribeCount;
void ExternalContextBase::Oversubscribe(....)
{
if (beginOversubscription)
{
ASSERT(m_oversubscribeCount >= 0);
++m_oversubscribeCount;
}
....
}Mensaje de diagnóstico de PVS-Studio:V547 La expresión 'm_oversubscribeCount>=0' siempre es verdadera. El valor de tipo sin firmar siempre es>=0. externalcontextbase.cpp 204
Y aquí está la lista de todos los demás problemas de este tipo:
- V547 La expresión 'm_oversubscribeCount>=0' siempre es verdadera. El valor de tipo sin firmar siempre es>=0. internalcontextbase.cpp 506
- V547 La expresión 'pGlobalNode->m_idleCores>=0' siempre es verdadera. El valor de tipo sin firmar siempre es>=0. resourcemanager.cpp 3764
- V547 La expresión 'pGlobalNode->m_disponibleCores>=0' siempre es verdadera. El valor de tipo sin firmar siempre es>=0. resourcemanager.cpp 3769
- V547 La expresión 'pReceiveProxyData->m_allocation>=0' siempre es verdadera. El valor de tipo sin firmar siempre es>=0. resourcemanager.cpp 4100
- V547 La expresión 'pReceiveProxyData->m_allocation>=0' siempre es verdadera. El valor de tipo sin firmar siempre es>=0. resourcemanager.cpp 4360
- V547 La expresión 'exclusiveCoresAvailable>=0' siempre es verdadera. El valor de tipo sin firmar siempre es>=0. resourcemanager.cpp 4657
- V547 La expresión 'coresNeeded>=0' siempre es verdadera. El valor de tipo sin firmar siempre es>=0. resourcemanager.cpp 4657
- V547 La expresión 'previousGlobal>=0' siempre es verdadera. El valor de tipo sin firmar siempre es>=0. resourcemanager.cpp 4781
- V547 La expresión 'currentGlobal>=0' siempre es verdadera. El valor de tipo sin firmar siempre es>=0. resourcemanager.cpp 4782
- V547 La expresión 'm_minConcurrency>=0' siempre es verdadera. El valor de tipo sin firmar siempre es>=0. schedulerproxy.cpp 63
- V547 La expresión 'm_minimumHardwareThreads>=0' siempre es verdadera. El valor de tipo sin firmar siempre es>=0. schedulerproxy.cpp 125
- V547 La expresión 'm_oversubscribeCount>=0' siempre es verdadera. El valor de tipo sin firmar siempre es>=0. umsthreadinternalcontext.cpp 308
- V547 La expresión 'j>=0' siempre es verdadera. El valor de tipo sin firmar siempre es>=0. resourcemanager.cpp 1922
- V547 La expresión 'pMaxNode->m_disponibleCores>=0' siempre es verdadera. El valor de tipo sin firmar siempre es>=0. resourcemanager.cpp 2542
- V547 La expresión 'previousLocal>=0' siempre es verdadera. El valor de tipo sin firmar siempre es>=0. resourcemanager.cpp 4793
- V547 La expresión 'currentLocal>=0' siempre es verdadera. El valor de tipo sin firmar siempre es>=0. resourcemanager.cpp 4794
- V547 La expresión siempre es verdadera. El valor de tipo sin firmar siempre es>=0. schedulerpolicybase.cpp 285
- V547 La expresión 'valor>=0' siempre es verdadera. El valor de tipo sin firmar siempre es>=0. schedulerpolicybase.cpp 345
Conversiones de tipos superfluos
He encontrado algunas conversiones de tipos explícitos que no solo son superfluas sino que también pueden estropear los valores.
Ejemplo uno:
size_t __cdecl strnlen(const char *str, size_t maxsize);
size_t __cdecl _mbstrnlen_l(const char *s,
size_t sizeInBytes,
_locale_t plocinfo)
{
....
if ( _loc_update.GetLocaleT()->locinfo->mb_cur_max == 1 )
/* handle single byte character sets */
return (int)strnlen(s, sizeInBytes);
....
}Mensaje de diagnóstico de PVS-Studio:V220 Secuencia sospechosa de conversión de tipos:memsize -> entero de 32 bits -> memsize. El valor que se está emitiendo:'strnlen(s, sizeInBytes)'. _mbslen_s.c 67
La función strnlen() devuelve un valor del tipo 'size_t'. Entonces, de repente, se convierte explícitamente en el tipo 'int'. Después de eso, el valor se extenderá implícitamente al tipo size_t.
Este código contiene un posible problema de 64 bits. Si uno intenta en una versión de programa de 64 bits calcular el número de caracteres en una cadena muy larga usando la función _mbstrnlen_l(), obtendrá un resultado incorrecto.
Supongo que esta conversión de tipo explícita se dejó en el código por accidente y solo debe eliminarse.
Ejemplo dos:
WINBASEAPI SIZE_T WINAPI GlobalSize (_In_ HGLOBAL hMem);
inline void __cdecl memcpy_s(
_Out_writes_bytes_to_(_S1max,_N) void *_S1,
_In_ size_t _S1max,
_In_reads_bytes_(_N) const void *_S2,
_In_ size_t _N);
AFX_STATIC HGLOBAL AFXAPI _AfxCopyGlobalMemory(....)
{
ULONG_PTR nSize = ::GlobalSize(hSource);
....
Checked::memcpy_s(lpDest, (ULONG)::GlobalSize(hDest),
lpSource, (ULONG)nSize);
....
}Mensaje de diagnóstico de PVS-Studio:V220 Secuencia sospechosa de conversión de tipos:memsize -> entero de 32 bits -> memsize. El valor que se está emitiendo:'nSize'. olemisc.cpp 684.
La función GlobalSize() devuelve el tipo SIZE_T. Los argumentos de la función memcpy_s() también tienen el tipo size_t.
Entonces, ¿para qué sirve la conversión de tipo explícita "(ULONG)::GlobalSize(hDest)"?
Si comenzamos a trabajar con un búfer de más de 4 Gb, la función memcpy_s() solo copiará una parte de la matriz.
Hay algunas otras conversiones de tipos sospechosos:
- V220 Secuencia sospechosa de conversión de tipos:memsize -> entero de 32 bits -> memsize. El valor que se está emitiendo:'wcslen(* vp ++)'. cenvarg.c 236
- V220 Secuencia sospechosa de conversión de tipos:memsize -> entero de 32 bits -> memsize. El valor que se está emitiendo:'::GlobalSize(m_hGlobalMemory)'. archivoshrd.cpp 48
- V220 Secuencia sospechosa de conversión de tipos:memsize -> entero de 32 bits -> memsize. El valor que se está emitiendo:'wcslen(lpsz)'. dumpcont.cpp 31
- V220 Secuencia sospechosa de conversión de tipos:memsize -> entero de 32 bits -> memsize. El valor que se está emitiendo:'wcslen(lpsz)'. dumpcont.cpp 82
- V220 Secuencia sospechosa de conversión de tipos:memsize -> entero de 32 bits -> memsize. El valor que se convierte:'(cElems * sizeof (CLSID))'. ctlcore.cpp 1975
- V220 Secuencia sospechosa de conversión de tipos:memsize -> entero de 32 bits -> memsize. El valor que se está emitiendo:'wParam'. afxtoolbarslistcheckbox.cpp 94
- V220 Secuencia sospechosa de conversión de tipos:memsize -> entero de 32 bits -> memsize. El valor que se está emitiendo:'nChars * sizeof (TCHAR)'. statreg.h 270
Uso de punteros antes de una verificación
CMFCPopupMenu* CMFCCustomizeButton::CreatePopupMenu()
{
....
if (m_pWndParentToolbar->IsLocked())
{
pMenu->GetMenuBar()->m_pRelatedToolbar = m_pWndParentToolbar;
}
pMenu->m_bRightAlign = m_bMenuRightAlign &&
(m_pWndParentToolbar->GetExStyle() & WS_EX_LAYOUTRTL) == 0;
BOOL bIsLocked = (m_pWndParentToolbar == NULL ||
m_pWndParentToolbar->IsLocked());
....
}Mensaje de diagnóstico de PVS-Studio:V595 El puntero 'm_pWndParentToolbar' se utilizó antes de que se verificara contra nullptr. Verificar líneas:192, 199. afxcustomizebutton.cpp 192
Primero se elimina la referencia del puntero 'm_pWndParentToolbar' en la expresión 'm_pWndParentToolbar->IsLocked()' y luego se comprueba si es nulo:'m_pWndParentToolbar ==NULL'.
Es un código peligroso y no creo que deba explicar por qué.
Otro caso así:
void COleControlSite::BindDefaultProperty(....)
{
....
if (pDSCWnd != NULL)
{
....
m_pDSCSite = pDSCWnd->m_pCtrlSite;
....
m_pDSCSite->m_pDataSourceControl->BindProp(this, TRUE);
if (m_pDSCSite != NULL)
m_pDSCSite->m_pDataSourceControl->BindColumns();
}
....
}Mensaje de diagnóstico de PVS-Studio:V595 El puntero 'm_pDSCSite' se utilizó antes de que se verificara contra nullptr. Verificar líneas:1528, 1529. occsite.cpp 1528
Variables superfluas
Las variables superfluas no son errores. Pero como son superfluos, uno todavía no los quiere en código y debe deshacerse de ellos. Por ejemplo:
int GetImageCount() const
{
CRect rectImage(m_Params.m_rectImage);
if (m_Bitmap.GetCount() == 1)
{
HBITMAP hBmp = m_Bitmap.GetImageWell();
BITMAP bmp;
if (::GetObject(hBmp, sizeof(BITMAP), &bmp) ==
sizeof(BITMAP))
{
return bmp.bmHeight / m_Params.m_rectImage.Height();
}
return 0;
}
return m_Bitmap.GetCount();
}Mensaje de diagnóstico de PVS-Studio:Se creó el objeto V808 'rectImage' de tipo 'CRect' pero no se utilizó. afxcontrolrenderer.h 89
El rectángulo 'rectImage' se crea pero no se usa de ninguna manera después de eso. Por lo tanto, tenemos una línea adicional en el programa y algunos ciclos de reloj de procesador adicionales para ejecutar cuando se trabaja con la versión de depuración.
Aquí hay un archivo con una lista de todas las variables superfluas:vs2003_V808.txt
Varios
Muchas advertencias de PVS-Studio señalan un estilo de codificación deficiente en lugar de errores. Mi opinión es que los códigos fuente de las bibliotecas de Visual C++ deberían servir como modelo a seguir para otros programadores y no es bueno enseñarles cosas malas.
A continuación se citan algunos fragmentos que se pueden mejorar.
Comparaciones peligrosas con VERDADERO
_PHNDLR __cdecl signal(int signum, _PHNDLR sigact)
{
....
if ( SetConsoleCtrlHandler(ctrlevent_capture, TRUE)
== TRUE )
....
}Mensaje de diagnóstico de PVS-Studio:V676 No es correcto comparar la variable de tipo BOOL con TRUE. winsig.c 255
Todas las fuentes, incluido MSDN, nos dicen que es una mala práctica comparar cualquier cosa con VERDADERO. La función puede devolver cualquier valor que no sea 0 y eso contará como VERDADERO. Pero VERDADERO es 1. Entonces, la forma correcta de calcular dicha comparación es Foo() !=FALSO.
Otras comparaciones similares:
- V676 Es incorrecto comparar la variable de tipo BOOL con TRUE. evento.cpp 448
- V676 Es incorrecto comparar la variable de tipo BOOL con TRUE. La expresión correcta es:'retVal !=FALSE'. administrador de recursos.cpp 1437
- V676 Es incorrecto comparar la variable de tipo BOOL con TRUE. La expresión correcta es:'retVal !=FALSE'. administrador de recursos.cpp 5027
Incremento
void _To_array(
::Concurrency::details::_Dynamic_array<_EType>& _Array)
{
_LockHolder _Lock(_M_lock);
_M_iteratorCount++;
for(_LinkRegistry::iterator _Link = _M_links.begin();
*_Link != NULL; _Link++)
{
_Array._Push_back(*_Link);
}
}Mensaje de diagnóstico de PVS-Studio:V803 Rendimiento reducido. En caso de que '_Link' sea iterador, es más efectivo usar la forma de incremento de prefijo. Reemplace iterador ++ con ++ iterador. agentes.h 1713
Es un matiz tan sutil, pero todas las fuentes recomiendan usar ++ iterador. Siempre que sea posible, es mejor usar un operador de prefijo como un buen estilo de codificación para que otros lo aprendan.
Nota. Algunas publicaciones sobre el tema:
- ¿Es razonable usar el operador de incremento de prefijo ++it en lugar del operador de postfijo it++ para los iteradores?
- Operador de incremento previo vs. posterior:punto de referencia.
Si los autores de las bibliotecas deciden que deberían trabajar en esos incrementos, aquí está la lista de todos los fragmentos que encontré:vs2003_V803.txt.
Restauración de estado de advertencia incorrecto
#pragma warning (disable : 4311)
SetClassLongPtr(m_hWnd,
GCLP_HBRBACKGROUND,
PtrToLong(reinterpret_cast<void*>(
::GetSysColorBrush(COLOR_BTNFACE))));
#pragma warning (default : 4311)El mensaje de diagnóstico V665:Posiblemente, el uso de 'advertencia #pragma (predeterminado:X)' es incorrecto en este contexto. En su lugar, se debe usar la 'advertencia #pragma (push/pop)'. Verifique las líneas:165, 167. afxbasepane.cpp 167
Una forma correcta de restaurar el estado de advertencia anterior es utilizar "#advertencia pragma(push[ ,n ])" y "#advertencia pragma(pop)".
Otros fragmentos similares:vs2003_V665.txt.
El cheque (este ==NULL)
Ese es un clásico del género:
_AFXWIN_INLINE CWnd::operator HWND() const
{ return this == NULL ? NULL : m_hWnd; }Mensaje de diagnóstico de PVS-Studio:se debe evitar la expresión V704 'this ==0'; esta expresión siempre es falsa en los compiladores más nuevos, porque el puntero 'this' nunca puede ser NULL. afxwin2.inl 19
Desafortunadamente, ese es un patrón muy común, especialmente en MFC. Pero los programadores deberían aprender gradualmente a dejar de usar construcciones como esa y, en su lugar, dar un buen ejemplo a los demás.
Aquellos que aún no saben por qué es malo, consulten la documentación sobre el diagnóstico V704 para obtener una explicación detallada.
Entiendo que el operador HWND() realmente no se puede arreglar:la compatibilidad con versiones anteriores es más importante. Pero, ¿por qué no hacer eso donde sea posible sin consecuencias dolorosas? Aquí está la lista de todas las comprobaciones de este tipo:vs2003_V704.txt
Conclusión
Como puede ver, el artículo resulta ser bastante grande. Pero en realidad no se encuentra nada demasiado interesante o crucial en las bibliotecas; su código es definitivamente de alta calidad y está bien depurado.
Estaré encantado si este artículo ayuda a mejorar un poco las bibliotecas de Visual C++ en el futuro. Permítanme señalar una vez más que lo que he hecho fue un análisis incompleto. Los desarrolladores de las bibliotecas de Visual C++ pueden realizar una mucho mejor y más completa, ya que tienen scripts/proyectos para construir las bibliotecas. Si tiene algún problema, estaré encantado de ayudarle a resolverlo. Póngase en contacto con nuestro servicio de asistencia.