logo linux

Otra bronca de Torvalds

Esta vez no solo se desahoga sino que resulta ser una bronca bastante didáctica en cuanto a conceptos de la programación en C.

La mala leche de Linus Torvalds es conocida por todos y él mismo ha reconocido en varias ocasiones que sus formas, aunque necesarias desde su punto de vista, pueden resultar chocantes a algunos.

En esta ocasión el enfado no solo está bastante justificado sino que además da algunas lecciones de C que ni los desarrolladores del kernel parecen conocer. Todo comenzó cuando un orgulloso desarrollador del kernel anunció los cambios relacionados con las redes a través de este mensaje:

Otra etapa de inclusión de características, otra serie de cambios en la redes. He oído que la infraestructura de túneles ligeros ha sido votada como cambio del año en redes.

A lo que Torvalds responde:

…y otros dicen que la característica más notable es el fallo imbécil que esta introduce y de la que incluso el compilador se queja.

Madre mía, gente. Aprended C en vez de introducir caracteres al azar hasta que compile (con advertencias).

Esto:

static bool rate_control_cap_mask(struct ieee80211_sub_if_data *sdata,
                                   struct ieee80211_supported_band *sband,
                                   struct ieee80211_sta *sta, u32 *mask,
                                   u8 mcs_mask[IEEE80211_HT_MCS_MASK_LEN])

Para empezar, esto está horriblemente mal porque los argumentos en arrays en realidad no existen en C. Desgraciadamente los compiladores aceptan esto por varias malas razones históricas y de forma silenciosa los convierte en el argumento de un puntero. Hay razonamientos para ello, pero vienen de mentes débiles.

Pero afortunadamente, gcc da advertencias muy válidas (elogios; a menudo acabo burlándome de las malas advertencias que da gcc pero en esta ocasión lo hace bien), porque unas lineas más abajo, el error se convierte en pura y absoluta basura.

Esta basura que básicamente surgió del primer error (pensar que C permite argumentos en arrays), es decir:

                  for (i = 0; i < sizeof(mcs_mask); i++)

«sizeof(mcs_mask)» es mierda. Debido a que los argumentos en arrays en realidad no existen en C, esto es el tamaño del punto, no del array. El primer error hace que el fallo parezca código razonable. Sin embargo, independientemente de eso argumentaría que el código en realidad estaría mal ya que «sizeof» es el tamaño en bytes y el código en realidad quiere el número de entradas (y para eso tenemos ARRAY_SIZE()).

Seguro que en este caso las entradas son sólo de un byte cada una así que esto debería haber «funcionado» de no ser por el asunto del argumento del array pero es engañoso y el código fundamentalmente es propenso a fallos y sin sentido de dos formas completamente diferentes y que se alimentan la una de la otra.

Esa linea se debería leer:

                  for (i = 0; i < IEEE80211_HT_MCS_MASK_LEN; i++)

y el argumento debería haber sido declarado como el puntero que es lo que es en realidad.

[…]

Resulta triste el hecho de que me haya dado cuenta de este fallo por un «simplemente compilemos cada propuesta y asegurémonos de que no es una completa mierda».

Ahora parece que el código se ha movido y el «sizeof()» inicialmente era correcto (porque era el tamaño de un array de verdad). Bien, esto era «correcto» en el sentido de que genera código correcto incluso si toda la confusión con «número de entradas» y «tamaño en bytes» todavía estuviera presente.

[…]

Así que veo cómo ha sucedido este fallo y sólo estoy ligeramente molesto con Lorenzo que es el autor de este código.

Lo que no veo es por qué el código ha existido en al menos dos árboles de mantenedores (el de Johannes y el de David) durante un par de semanas y a nadie le ha importado las nuevas advertencias del compilador. ¿Y me fue enviada a pesar de la nueva advertencia?

[…]

Por favor, gente. Cuando veo este tipo de código con fallos obvios, me molesta mucho. Porque eso hace que me preocupe por todas esas cosas no tan obvias que me pierdo. Desgraciadamente esta vez he añadido el código pronto (porque quería probar los cambios en las redes inalámbricas en mi portátil) así que ahora el fallo está ahí fuera.

No estoy seguro del impacto real de este fallo. […] Puede que las pruebas no muestren este fallo pero me gustaría que la gente se tomara más en serio las advertencias del compilador (y fueran mucho más cuidadosos a la hora de mover cosas a funciones y nunca jamás usar el modelo «argumentos de una función es un array»).

Linus.

Esta respuesta deja claro que el control de calidad del kernel a veces deja bastante que desear y también muestra cómo, a veces, las formas de Torvalds están justificadas. Que existan este tipo de fallos y malas prácticas en la programación y que dos equipos enteros del kernel no se den cuenta, da en qué pensar.

Si quieres ver lo que opina Torvalds de ciertas contribuciones, echa un vistazo a su respuesta a este desarrollador.

Podéis consultar el mensaje en la lista de correo del kernel.