Algum tempo atrás eu vi um código muito errado, mas muito errado mesmo, e de muitas formas diferentes, que me deixou chocado.
Não vou colocar o código original aqui, mas vou refazer usando o mesmo algoritmo usando outra situação.
Situação nova: Em alguns países o ponto é usado para separar as decimais e a vírgula é usada a cada 3 casas não decimais, mas em outros, como o Brasil, é ao contrário.
Por exemplo, nos EUA a velocidade da luz seria escrita como 1,079,252,848.8 Km/h (Eu sei, eles não usam Km/h, e sim aquela medida obsoleta de milhas por hora.), e no Brasil este número seria representado assim: 1.079.252.848,8 Km/h.
E no caso seria necessária uma função que fizesse esta troca e colocasse o "c = " na frente. A função seria escrita deste modo abaixo com o algoritmo que vi:
Agora analise o código. Veja quantos erros tem nele. Imagine as consequências destes erros. Imagine como resolvê-los. Depois continue a ler.
Já listou os erros que você viu? Vou listar os que vi.
É uma string, e não uma array
O parâmetro texto está sendo tratado como um array com o tamanho fixo de 50 caracteres, e não como uma string terminada por nulo.
Esta string pode ter mais de 50 caracteres, e facilmente ter menos de 50 caracteres. Portanto pode não varrer a string toda, como pode passar além da string.
Consequências de ultrapassar a string
Ao ultrapassar o nulo final da string podem acontecer duas coisas.
- Se esta string, mesmo com menos de 50 caracteres, se ela tiver os 50 bytes alocados, terá ocorrido um desperdício de processamento.
- Se a alocação dela for menor do que 50 caracteres (só o tamanho mínimo necessário, por exemplo.), o prosseguimento da varredura poderá achar uma vírgula e/ou um ponto em algo que não era realmente parte da string e fazer as trocas. As consequências disso podem ser terríveis.
Exemplos das possíveis consequências seriam os números inteiros 300 e 302. O número 300 é representado em um byte com 1, e no outro por 44, e o 302 por 1 e 46, sendo respectivamente 256*1+1*44 e 256*1+1*46. Mas 44 é a vírgula na tabela ASCII e 46 é o ponto. Com a troca um número 300 seria mudado para 302 e vice-versa.
Acha esta mudança pequena? Vamos mudar o exemplo para o número inteiro 11310. Ele é 44*256+46. Ele seria trocado para 11820, que é 46*256+44. As consequências disso podem ser desastrosas.
Se pegar o expoente de um número de ponto flutuante o disparate pode ser pior ainda. Muda o valor em ordem de grandeza.
Mas os problemas não param por aí. Outras strings podem estar armazenadas perto que foi passada por parâmetro, e elas poderiam ser modificadas também, podendo mudar significados de mensagens, e até mudar as casas decimais de outros números representados em texto.
Por exemplo, "A TV custa 1.500 Reais" mudaria para a custar 1,500 Reais, ou seja R$ 1,50.
Mas se o parâmetro aponta para uma variável na stack, como acontece normalmente com variáveis locais das funções, as consequências podem ser mais nefastas ainda. Pode vir a adulterar o endereço de retorno de uma função, levando a um retorno no lugar errado. As consequências podem ser mais imprevisíveis ainda.
Diferenças de tamanho
Digamos que a situação está muito bem controlada. Que a string passada como parâmetro sempre tem os 50 bytes alocados, podendo ter até 49 caracteres (dígitos, pontos e vírgulas) e mais o nulo.
Pode ter a perda de desempenho mencionada antes, caso a string seja menor, mas ultrapassará o tamanho reservado para temp na hora do strcat() se for toda, ou quase toda, usada.
Se a string recebida como parâmetro tiver 49 dígitos, usando os 50 caracteres, ao ser concatenada com temp, que já tem 3, dará 53 caracteres, mas está alocado só 50.
O excedente passará por cima de alguma coisa na stack, coisa que pode ser desastrosa, como mencionado antes.
Mas não para por aí, pois esta violação de tamanho também é replicada para a string original com o strcpy() no final.
Muitas operações de varredura e cópia
Neste algoritmo é feita uma varredura, depois o strcat() que varre uma string para achar o final dela, e uma varredura da outra copiando para a primeira. Depois é feita mais uma varredura copiando para o local original com a strcpy().
Não seria melhor ter menos operações? Sim, tem como.
Alteração da string original
A string original, a passada como parâmetro, é alterada, pois a resposta da função é na própria string recebida como parâmetro, e o programador que for usar esta função tem que estar ciente disso. Se o original tem que ser preservada, ele terá que passar uma cópia.
Esta função nunca poderá ser usada da seguinte forma:
Troca( "1,079,252,848.8 Km/h" );
Não tem nenhum ponteiro apontando pra string passada como parâmetro, e alguns compiladores colocam estas strings constantes na área de código (Exceto quando pedido em contrário por opção de compilação.), área que não pode ser modificada. Então o programa abortaria na execução na primeira vírgula ou ponto achado, ou no strcpy() do final.
O não uso de ponteiros
Este é um caso que poderia ser melhor resolvido usando ponteiros na varredura da string recebida, coisa que mostrarei adiante. Geraria menos indexação, mas bons compiladores talvez pudessem resolver este problema na otimização de código.
Conclusões sobre o algoritmo
Este algoritmo parece ter sido feito por alguém que não sabia como realmente um programa roda. Não sabendo ver as consequências do código que escreve, e mal sabe usar strings em C.
A impressão que me passa é que foi escrito por alguém que ainda estava aprendendo a programar.
Soluções
Eu vejo 3 soluções neste momento. Em todas o parâmetro é preservado e é feita uma única varredura
Cada uma tem vantagens e desvantagens, dependendo da situação, e a terceira talvez seja a melhor de todas.
// Necessario por causa da malloc().
#include <stdlib.h>
// Necessario por causa da strlen().
#include <string.h>
char *Troca1( char *texto )
{
// Problema: A alocacao tem que ser suficiente, mas tem
{
*(p++) = 'c' ;
*(p++) = ' ' ;
Na primeira solução, Troca1, existe uma área de memória alocada a tempo de compilação, o que pode ser muito rápida, já que já começa inicializada e com o "c = " do tempo de compilação. Mas tem o problema de poder estar superdimencionada, ou subdimencionada.
Na segunda solução, Troca2, a resposta é alocada em tempo de execução, mas o programador não pode esquecer de desalocar depois sob pena de causar perda de memória, memory leak. Tem um gasto de processamento na alocação de memória e para saber o tamanho da string original.
A terceira solução é a melhor, creio eu, pois em cada uso poderá alocar de uma forma diferente, conforme necessidade e preferência. Mas o programador tem que estar ciente que o espaço para a resposta tem que caber o original e mais o "c = ".
Uma pergunta que não quer calar
Como um erro destes passou escondido no código por mais de uma década sem causar um desastre? Isso me deixou encucado, intrigado.
Quando eu ia consertar resolvi ver os lugares onde a função era usada, e descobri que não era usada. Isto explica bem o fato de nunca ter dado problemas. Então não consertei, mas coloquei um comentário na função informado que ela estava muito errada e era para ser refeita para poder ser usada.
Sobre o teste
O programa com as 3 soluções apresentado acima foi testado no seguinte compilador:
FreeBSD clang version 13.0.0 (git@github.com:llvm/llvm-project.git llvmorg-13.0.0-0-gd7b669b3a303)
Target: x86_64-unknown-freebsd12.3
Thread model: posix
InstalledDir: /usr/bin
A resposta foi:
c = 1.079.252.848,8 Km/h
c = 299.792.458 m/s
c = 1.079.252.848,8 Km/h
c = 299.792.458 m/s
c = 1.079.252.848,8 Km/h
c = 299.792.458 m/s
Não foi feito nenhum teste de desempenho comparando as 3 soluções
Finalizando
Se viu mais problemas do que eu listei no algoritmo que critico, e/ou tem soluções melhores do que as minhas, ou alguma sugestão de melhoria para elas, coloque nos comentários.
Nenhum comentário:
Postar um comentário