Este es un segundo artículo, que se centra en el uso del analizador PVS-Studio en sistemas CI en la nube. Esta vez consideraremos la plataforma Azure DevOps, una solución de CI \ CD en la nube de Microsoft. Analizaremos el proyecto ShareX.
Necesitaremos tres componentes. El primero es el analizador PVS-Studio. El segundo es Azure DevOps, con el que integraremos el analizador. El tercero es el proyecto que revisaremos para demostrar las habilidades de PVS-Studio cuando trabaje en una nube. Así que vámonos.
PVS-Studio es un analizador de código estático para encontrar errores y defectos de seguridad. La herramienta admite el análisis de código C, C ++ y C #.
Azure DevOps . La plataforma Azure DevOps incluye herramientas como Azure Pipeline, Azure Board, Azure Artifacts y otras que aceleran el proceso de creación de software y mejoran su calidad.
ShareX es una aplicación gratuita que te permite capturar y grabar cualquier parte de la pantalla. El proyecto está escrito en C # y es muy adecuado para mostrar la configuración del lanzamiento del analizador estático. El código fuente del proyecto está
disponible en GitHub .
La salida del comando cloc para el proyecto ShareX:
En otras palabras, el proyecto es pequeño, pero suficiente para demostrar el trabajo de PVS-Studio junto con la plataforma en la nube.
Comencemos la configuración
Para comenzar a trabajar en Azure DevOps, sigamos el
enlace y presione "Comenzar gratis con GitHub".
Otorgue a la aplicación de Microsoft acceso a los datos de la cuenta de GitHub.
Tendrá que crear una cuenta de Microsoft para completar su registro.
Después del registro, cree un proyecto:
Luego, debemos pasar a "Tuberías" - "Construcciones" y crear una nueva tubería de construcción.
Cuando se nos pregunte dónde se encuentra nuestro código, responderemos: GitHub.
Autorice Azure Pipelines y elija el repositorio con el proyecto, para el cual configuraremos la ejecución del analizador estático.
En la ventana de selección de plantilla, elija "Canalización de inicio".
Podemos ejecutar análisis de código estático del proyecto de dos maneras: usando agentes alojados en Microsoft o autohospedados.
Primero, utilizaremos agentes alojados en Microsoft. Dichos agentes son máquinas virtuales ordinarias que se lanzan cuando ejecutamos nuestra tubería. Se eliminan cuando se realiza la tarea. El uso de dichos agentes nos permite no perder tiempo para su soporte y actualización, pero impone ciertas restricciones, por ejemplo, la incapacidad de instalar software adicional que se utiliza para construir un proyecto.
Reemplacemos la configuración predeterminada sugerida por la siguiente para usar agentes alojados en Microsoft:
# Setting up run triggers # Run only for changes in the master branch trigger: - master # Since the installation of random software in virtual machines # is prohibited, we'll use a Docker container, # launched on a virtual machine with Windows Server 1803 pool: vmImage: 'win1803' container: microsoft/dotnet-framework:4.7.2-sdk-windowsservercore-1803 steps: # Download the analyzer distribution - task: PowerShell@2 inputs: targetType: 'inline' script: 'Invoke-WebRequest -Uri https://files.viva64.com/PVS-Studio_setup.exe -OutFile PVS-Studio_setup.exe' - task: CmdLine@2 inputs: workingDirectory: $(System.DefaultWorkingDirectory) script: | # Restore the project and download dependencies nuget restore .\ShareX.sln # Create the directory, where files with analyzer reports will be saved md .\PVSTestResults # Install the analyzer PVS-Studio_setup.exe /VERYSILENT /SUPPRESSMSGBOXES /NORESTART /COMPONENTS=Core # Create the file with configuration and license information "C:\Program Files (x86)\PVS-Studio\PVS-Studio_Cmd.exe" credentials -u $(PVS_USERNAME) -n $(PVS_KEY) # Run the static analyzer and convert the report in html. "C:\Program Files (x86)\PVS-Studio\PVS-Studio_Cmd.exe" -t .\ShareX.sln -o .\PVSTestResults\ShareX.plog "C:\Program Files (x86)\PVS-Studio\PlogConverter.exe" -t html -o .\PVSTestResults\ .\PVSTestResults\ShareX.plog # Save analyzer reports - task: PublishBuildArtifacts@1 inputs: pathToPublish: PVSTestResults artifactName: PVSTestResults
Nota: de acuerdo con la
documentación , el contenedor utilizado debe almacenarse en caché en la imagen de la máquina virtual, pero al momento de escribir el artículo no funciona y el contenedor se descarga cada vez que se inicia la tarea, lo que tiene un impacto negativo en El tiempo de ejecución.
Guardemos la tubería y creemos variables que se usarán para crear el archivo de licencia. Para hacer esto, abra la ventana de edición de tubería y haga clic en "Variables" en la esquina superior derecha.
Luego, agregue dos variables:
PVS_USERNAME y
PVS_KEY , que contienen el nombre de usuario y la clave de licencia, respectivamente. Al crear la variable
PVS_KEY , no olvide seleccionar "Mantener este valor en secreto" para cifrar los valores de la variable con una clave RSA de 2048 bits y suprimir la salida del valor de la variable en el registro de rendimiento de la tarea.
Guarde las variables y ejecute la canalización haciendo clic en "Ejecutar".
La segunda opción para ejecutar el análisis: usar un agente autohospedado. Podemos personalizar y administrar agentes autohospedados nosotros mismos. Dichos agentes brindan más oportunidades para instalar el software, necesario para construir y probar nuestro producto de software.
Antes de usar dichos agentes, debe configurarlos de acuerdo con las
instrucciones e instalar y
configurar el analizador estático.
Para ejecutar la tarea en un agente autohospedado, reemplazaremos la configuración sugerida con lo siguiente:
# Setting up triggers # Run the analysis for master-branch trigger: - master # The task is run on a self-hosted agent from the pool 'MyPool' pool: 'MyPool' steps: - task: CmdLine@2 inputs: workingDirectory: $(System.DefaultWorkingDirectory) script: | # Restore the project and download dependencies nuget restore .\ShareX.sln # Create the directory where files with analyzer reports will be saved md .\PVSTestResults # Run the static analyzer and convert the report in html. "C:\Program Files (x86)\PVS-Studio\PVS-Studio_Cmd.exe" -t .\ShareX.sln -o .\PVSTestResults\ShareX.plog "C:\Program Files (x86)\PVS-Studio\PlogConverter.exe" -t html -o .\PVSTestResults\ .\PVSTestResults\ShareX.plog # Save analyzer reports - task: PublishBuildArtifacts@1 inputs: pathToPublish: PVSTestResults artifactName: PVSTestResults
Una vez que se complete la tarea, puede descargar el archivo con los informes del analizador en la pestaña "Resumen" o puede usar la extensión
Enviar correo que permite configurar el correo electrónico o considerar otra herramienta conveniente en
Marketplace .
Resultados de analisis
Ahora veamos algunos errores encontrados en el proyecto probado, ShareX.
Controles excesivosPara calentar, comencemos con fallas simples en el código, es decir, con verificaciones redundantes:
private void PbThumbnail_MouseMove(object sender, MouseEventArgs e) { .... IDataObject dataObject = new DataObject(DataFormats.FileDrop, new string[] { Task.Info.FilePath }); if (dataObject != null) { Program.MainForm.AllowDrop = false; dragBoxFromMouseDown = Rectangle.Empty; pbThumbnail.DoDragDrop(dataObject, DragDropEffects.Copy | DragDropEffects.Move); Program.MainForm.AllowDrop = true; } .... }
Advertencia de PVS-Studio: V3022 [CWE-571] La expresión 'dataObject! = Null' siempre es verdadera. TaskThumbnailPanel.cs 415
Prestemos atención a la comprobación de la variable
dataObject para
null . ¿Por qué está aquí?
dataObject no puede ser
nulo en este caso, ya que se inicializa con una referencia en un objeto creado. Como resultado, tenemos un control excesivo. Crítico? No Parece sucinto? No Esta comprobación es claramente mejor eliminarla para no saturar el código.
Veamos otro fragmento de código que podemos comentar de manera similar:
private static Image GetDIBImage(MemoryStream ms) { .... try { .... return new Bitmap(bmp); .... } finally { if (gcHandle != IntPtr.Zero) { GCHandle.FromIntPtr(gcHandle).Free(); } } .... } private static Image GetImageAlternative() { .... using (MemoryStream ms = dataObject.GetData(format) as MemoryStream) { if (ms != null) { try { Image img = GetDIBImage(ms); if (img != null) { return img; } } catch (Exception e) { DebugHelper.WriteException(e); } } } .... }
Advertencia de PVS-Studio: V3022 [CWE-571] La expresión 'img! = Null' siempre es verdadera. ClipboardHelpers.cs 289
En el método
GetImageAlternative , se verifica que la variable
img no sea nula justo después de crear una nueva instancia de la clase
Bitmap . La diferencia con el ejemplo anterior aquí es que usamos el método
GetDIBImage en lugar del constructor para inicializar la variable
img . El autor del código sugiere que podría ocurrir una excepción en este método, pero declara que solo bloquea el
intento y
finalmente , omite la
captura . Por lo tanto, si se produce una excepción, el método de llamada
GetImageAlternative no obtendrá una referencia a un objeto del tipo
Bitmap , pero tendrá que manejar la excepción en su propio bloque
catch . En este caso, la variable
img no se inicializará y el hilo de ejecución ni siquiera alcanzará la verificación
img! = Null , pero entrará en el bloque catch. En consecuencia, el analizador señaló un control excesivo.
Consideremos el siguiente ejemplo de una advertencia
V3022 :
private void btnCopyLink_Click(object sender, EventArgs e) { .... if (lvClipboardFormats.SelectedItems.Count == 0) { url = lvClipboardFormats.Items[0].SubItems[1].Text; } else if (lvClipboardFormats.SelectedItems.Count > 0) { url = lvClipboardFormats.SelectedItems[0].SubItems[1].Text; } .... }
Advertencia de PVS-Studio: V3022 [CWE-571] La expresión 'lvClipboardFormats.SelectedItems.Count> 0' siempre es verdadera. AfterUploadForm.cs 155
Echemos un vistazo más de cerca a la segunda expresión condicional. Allí verificamos el valor de la propiedad
Count de solo lectura. Esta propiedad muestra el número de elementos en la instancia de la colección
SelectedItems . La condición solo se ejecuta si la propiedad
Count es mayor que cero. Todo estaría bien, pero en el externo
si la instrucción
Count ya está marcada para 0. La instancia de la colección
SelectedItems no puede tener el número de elementos menor que cero, por lo tanto,
Count es igual o mayor que 0. Dado que hemos ya realizó la verificación de
Conteo para 0 en la primera instrucción
if y era falsa, no tiene sentido escribir otra verificación de
Conteo por ser mayor que cero en la rama else.
El último ejemplo de una advertencia
V3022 será el siguiente fragmento de código:
private void DrawCursorGraphics(Graphics g) { .... int cursorOffsetX = 10, cursorOffsetY = 10, itemGap = 10, itemCount = 0; Size totalSize = Size.Empty; int magnifierPosition = 0; Bitmap magnifier = null; if (Options.ShowMagnifier) { if (itemCount > 0) totalSize.Height += itemGap; .... } .... }
Advertencia de PVS-Studio: V3022 La expresión 'itemCount> 0' siempre es falsa. RegionCaptureForm.cs 1100
El analizador notó que la condición
itemCount> 0 siempre será falsa, ya que la variable
itemCount se declara y al mismo tiempo se le asigna cero arriba. Esta variable no se usa en ningún lugar hasta la misma condición, por lo tanto, el analizador tenía razón sobre la expresión condicional, cuyo valor siempre es falso.
Bueno, veamos ahora algo realmente sagaz.
La mejor manera de entender un error es visualizarloNos parece que se encontró un error bastante interesante en este lugar:
public static void Pixelate(Bitmap bmp, int pixelSize) { .... float r = 0, g = 0, b = 0, a = 0; float weightedCount = 0; for (int y2 = y; y2 < yLimit; y2++) { for (int x2 = x; x2 < xLimit; x2++) { ColorBgra color = unsafeBitmap.GetPixel(x2, y2); float pixelWeight = color.Alpha / 255; r += color.Red * pixelWeight; g += color.Green * pixelWeight; b += color.Blue * pixelWeight; a += color.Alpha * pixelWeight; weightedCount += pixelWeight; } } .... ColorBgra averageColor = new ColorBgra((byte)(b / weightedCount), (byte)(g / weightedCount), (byte)(r / weightedCount), (byte)(a / pixelCount)); .... }
No me gustaría mostrar todas las tarjetas y revelar lo que nuestro analizador ha encontrado, así que dejémoslo a un lado por un tiempo.
Por el nombre del método, es fácil adivinar lo que está haciendo: le das una imagen o un fragmento de una imagen, y la pixela. El código del método es bastante largo, por lo que no lo citaremos por completo, solo trataremos de explicar su algoritmo y explicar qué tipo de error logró encontrar PVS-Studio.
Este método recibe dos parámetros: un objeto del tipo
Bitmap y el valor del tipo
int que indica el tamaño de la pixelación. El algoritmo de operación es bastante simple:
1) Divida el fragmento de imagen recibido en cuadrados con el lado igual al tamaño de pixelación. Por ejemplo, si tenemos un tamaño de pixelación igual a 15, obtendremos un cuadrado que contiene 15x15 = 225 píxeles.
2) Además, atravesamos cada píxel en este cuadrado y acumulamos los valores de los campos
Rojo ,
Verde ,
Azul y
Alfa en variables intermedias, y antes de eso multiplicamos el valor del color correspondiente y el canal alfa por la variable
pixelWeight , obtenida por dividiendo el valor
Alpha por 255 (la variable
Alpha es del tipo
byte ). Además, al atravesar píxeles
sumamos los valores, escritos en
pixelWeight en la variable
weightedCount . El fragmento de código que ejecuta las acciones anteriores es el siguiente:
ColorBgra color = unsafeBitmap.GetPixel(x2, y2); float pixelWeight = color.Alpha / 255; r += color.Red * pixelWeight; g += color.Green * pixelWeight; b += color.Blue * pixelWeight; a += color.Alpha * pixelWeight; weightedCount += pixelWeight;
Por cierto, tenga en cuenta que si el valor de la variable
Alfa es cero,
pixelWeight no agregará a la variable
weightedCount ningún valor para este píxel. Lo necesitaremos en el futuro.
3) Después de atravesar todos los píxeles en el cuadrado actual, podemos hacer un color "promedio" común para este cuadrado. El código que hace esto tiene el siguiente aspecto:
ColorBgra averageColor = new ColorBgra((byte)(b / weightedCount), (byte)(g / weightedCount), (byte)(r / weightedCount), (byte)(a / pixelCount));
4) Ahora, cuando obtuvimos el color final y lo escribimos en la variable
averageColor , podemos atravesar nuevamente cada píxel del cuadrado y asignarle un valor de
averageColor .
5) Regrese al punto 2 mientras tenemos cuadrados sin manejar.
Una vez más, la variable
weightedCount no es igual al número de todos los píxeles en un cuadrado. Por ejemplo, si una imagen contiene un píxel completamente transparente (valor cero en el canal alfa), la variable
pixelWeight será cero para este píxel (
0/255 = 0). Por lo tanto, este píxel no afectará la formación de la variable
weightedCount . Es bastante lógico: no tiene sentido tener en cuenta los colores de un píxel completamente transparente.
Entonces todo parece razonable: la pixelación debe funcionar correctamente. Y en realidad lo hace. Eso no es para imágenes png que incluyen píxeles con valores en el canal alfa por debajo de 255 y desiguales a cero. Observe la imagen pixelada a continuación:
¿Has visto la pixelación? Nosotros tampoco. Bien, ahora vamos a revelar esta pequeña intriga y explicar dónde se esconde exactamente el error en este método. El error se deslizó en la línea del
cálculo de la variable
pixelWeight :
float pixelWeight = color.Alpha / 255;
El hecho es que al declarar la variable
pixelWeight como
flotante , el autor del código implicó que al dividir el campo
Alfa entre 255, obtendrá números fraccionarios además de cero y uno. Aquí es donde se esconde el problema, ya que la variable
Alpha es del tipo
byte . Al sumergirlo en 255, obtenemos un valor entero. Solo después de eso, se lanzará implícitamente al tipo
flotante , lo que significa que la parte fraccional se pierde.
Es fácil explicar por qué es imposible pixelar imágenes png con cierta transparencia. Dado que para estos píxeles los valores del canal alfa están en el rango 0 <Alfa <255, la variable
Alfa dividida por 255 siempre dará como resultado 0. Por lo tanto, los valores de las variables
pixelWeight ,
r ,
g ,
b ,
a ,
weightedCount también siempre será 0. Como resultado, nuestro
Color promedio estará con valores cero en todos los canales: rojo - 0, azul - 0, verde - 0, alfa - 0. Al pintar un cuadrado en este color, no cambiamos el color original de los píxeles, ya que el
color promedio es absolutamente transparente. Para corregir este error, solo necesitamos emitir explícitamente el campo
Alfa al tipo
flotante . La versión fija de la línea de código podría verse así:
float pixelWeight = (float)color.Alpha / 255;
Bueno, ya es hora de citar el mensaje de PVS-Studio para el código incorrecto:
Advertencia de PVS-Studio: V3041 [CWE-682] La expresión se
convirtió implícitamente de tipo 'int' a tipo 'flotante'. Considere utilizar un molde de tipo explícito para evitar la pérdida de una parte fraccional. Un ejemplo: doble A = (doble) (X) / Y;. ImageHelpers.cs 1119
A modo de comparación, citemos la captura de pantalla de una imagen verdaderamente pixelada, obtenida en la versión de aplicación corregida:
Potencial NullReferenceException public static bool AddMetadata(Image img, int id, string text) { .... pi.Value = bytesText; if (pi != null) { img.SetPropertyItem(pi); return true; } .... }
Advertencia de PVS-Studio: V3095 [CWE-476] El objeto 'pi' se usó antes de que se verificara como nulo. Verifique las líneas: 801, 803. ImageHelpers.cs 801
Este fragmento de código muestra que el autor esperaba que la variable
pi pueda ser
nula , es por eso que antes de llamar al método
SetPropertyItem , se realiza la comprobación
pi! = Null . Es extraño que antes de esta comprobación, a la propiedad se le asigne una matriz de bytes, porque si
pi es
nulo , se generará una excepción del tipo
NullReferenceException .
Una situación similar se ha notado en otro lugar:
private static void Task_TaskCompleted(WorkerTask task) { .... task.KeepImage = false; if (task != null) { if (task.RequestSettingUpdate) { Program.MainForm.UpdateCheckStates(); } .... } .... }
Advertencia de PVS-Studio: V3095 [CWE-476] El objeto 'tarea' se usó antes de que se verificara como nulo. Líneas de verificación: 268, 270. TaskManager.cs 268
PVS-Studio encontró otro error similar. El punto es el mismo, por lo que no hay una gran necesidad de citar el fragmento de código, el mensaje del analizador será suficiente.
Advertencia de PVS-Studio: V3095 [CWE-476] El objeto 'Config.PhotobucketAccountInfo' se usó antes de que se verificara como nulo. Líneas de verificación: 216, 219. UploadersConfigForm.cs 216
El mismo valor de retornoSe encontró un fragmento de código sospechoso en el método
EvalWindows de la clase
WindowsList , que devuelve
verdadero en todos los casos:
public class WindowsList { public List<IntPtr> IgnoreWindows { get; set; } .... public WindowsList() { IgnoreWindows = new List<IntPtr>(); } public WindowsList(IntPtr ignoreWindow) : this() { IgnoreWindows.Add(ignoreWindow); } .... private bool EvalWindows(IntPtr hWnd, IntPtr lParam) { if (IgnoreWindows.Any(window => hWnd == window)) { return true;
Advertencia de PVS-Studio: V3009 Es extraño que este método siempre devuelva el mismo valor de 'verdadero'. WindowsList.cs 82
Parece lógico que si en la lista llamada
IgnoreWindows hay un puntero con el mismo nombre que
hWnd , el método debe devolver
falso .
La lista
IgnoreWindows se puede completar cuando se llama al constructor
WindowsList (IntPtr ignoreWindow) o directamente a través del acceso a la propiedad como pública. De todos modos, según Visual Studio, en este momento en el código esta lista no está llena. Este es otro lugar extraño de este método.
Llamada insegura de controladores de eventos protected void OnNewsLoaded() { if (NewsLoaded != null) { NewsLoaded(this, EventArgs.Empty); } }
Advertencia de PVS-Studio: V3083 [CWE-367] Invocación insegura del evento 'NewsLoaded', NullReferenceException es posible. Considere asignar un evento a una variable local antes de invocarlo. NewsListControl.cs 111
Aquí puede ocurrir un caso muy desagradable. Después de comprobar que la variable
NewsLoaded es nula, el método, que maneja un evento, puede darse de baja, por ejemplo, en otro hilo. En este caso, cuando lleguemos al cuerpo de la instrucción if, la variable
NewsLoaded ya será nula. Se puede producir una
excepción NullReferenceException al intentar llamar a los suscriptores del evento
NewsLoaded , que es nulo. Es mucho más seguro usar un operador condicional nulo y reescribir el código anterior de la siguiente manera:
protected void OnNewsLoaded() { NewsLoaded?.Invoke(this, EventArgs.Empty); }
El analizador señaló
68 fragmentos similares. No los describiremos a todos: todos tienen un patrón de llamada similar.
Devolver nulo de ToStringRecientemente descubrí en un
interesante artículo de mi colega que Microsoft no recomienda devolver nulo del método anulado
ToString . PVS-Studio es muy consciente de esto:
public override string ToString() { lock (loggerLock) { if (sbMessages != null && sbMessages.Length > 0) { return sbMessages.ToString(); } return null; } }
Advertencia de PVS-Studio: V3108 No se recomienda devolver 'nulo' del método 'ToSting ()'. Logger.cs 167
¿Por qué se asigna si no se usa? public SeafileCheckAccInfoResponse GetAccountInfo() { string url = URLHelpers.FixPrefix(APIURL); url = URLHelpers.CombineURL(APIURL, "account/info/?format=json"); .... }
Advertencia de PVS-Studio: V3008 A la variable 'url' se le asignan valores dos veces seguidas. Quizás esto sea un error. Líneas de verificación: 197, 196. Seafile.cs 197
Como podemos ver en el ejemplo, al declarar la variable
url , se le asigna un valor, devuelto por el método
FixPrefix . En la siguiente línea, borramos el valor obtenido incluso sin usarlo en ningún lado. Obtenemos algo similar al código muerto: funciona, pero no afecta el resultado. Lo más probable es que este error sea el resultado de copiar y pegar, ya que dichos fragmentos de código tienen lugar en 9 métodos más. Como ejemplo, citaremos dos métodos con una primera línea similar:
public bool CheckAuthToken() { string url = URLHelpers.FixPrefix(APIURL); url = URLHelpers.CombineURL(APIURL, "auth/ping/?format=json"); .... } .... public bool CheckAPIURL() { string url = URLHelpers.FixPrefix(APIURL); url = URLHelpers.CombineURL(APIURL, "ping/?format=json"); .... }
Conclusiones
Como podemos ver, la complejidad de la configuración de las comprobaciones automáticas del analizador no depende del sistema de CI elegido. Nos tomó literalmente 15 minutos y varios clics del mouse para configurar la verificación de nuestro código de proyecto con un analizador estático.
En conclusión, lo invitamos a
descargar y probar el analizador en sus proyectos.