Commentaires sur copy.c
Le corrigé dont il est question est très imparfait.
Voici des commentaires faits par Thomas, Marc, Nicolas et David
permettant de voir la différence en un code qui apparemment marche et un
code robuste.
Syntaxe du C et style de codage
- usage est déclarée sans prototype.
- La variable flag est mal nommée.
- Les messages d'erreur "à la main" sont envoyés sur stdout, via
puts(), mais ceux de perror() vont sur stderr.
- Tes perror ne disent pas ce qui a échoué. Si je me prends un no such
file or directory, je ne sais pas si c'est le fichier source qui
n'existe pas, ou le répertoire dans lequel je peux créer la cible. Idem
pour un permission denied.
- Tu protèges les appels à mmap() avec _POSIX_MAPPED_FILES, mais tu
ne protèges pas le #include . Si l'OS ne gère pas mmap(),
ton programme peut très bien ne pas compiler.
- Tu utilises strcmp() et memcpy() sans avoir inclus .
Honte sur toi.
- Ca manque de const sur tes declarations de tableaux...
- Le fichier de sortie est ouvert en écriture _et_ en lecture, ce qui
est inutile.
- Les constantes 0 et 1 passées à exit() devraient être EXIT_SUCCESS
et EXIT_FAILURE, respectivement (déclarées dans )(certes,
ceci n'est important que si on porte à un système non-Unix).
- L'ensemble est inutilement spécifique Unix. La version sans mmap()
pourrait être écrite en C standard, avec fopen(), fread(), fwrite() et
fclose(), et serait portable sur d'autres OS tels que Ms-Dos. De plus,
les implémentations usuelles de fwrite() font le test d'EINTR et les
boucles nécessaires.
Codes de retour
- Tu ne testes pas bien les codes d'erreurs ! Il te faudrait vérifier
que quand open(), read(), write() et close() renvoient -1, errno n'est
pas à EINTR. Là, tu échoues sur des conditions pourtant pleinement
valides.
- Un joli bug à ce sujet :
do {
if (howmany = read(fd, buffer, CPBUFLEN))
if (howmany != write(ofd, buffer, howmany)) {
perror("");
exit(1);
}
} while (howmany);
Je passe sur l'affectation utilisée directement comme test : c'est du
"poor style" mais c'est légal. Je passe rapidement aussi sur le fait que
tu échoues sur les write() partiels, qui peuvent pourtant arriver sans
que ce soit une erreur.
Mais si read() te renvoie -1 (par suite d'un signal reçu où d'une
erreur quelconque), alors tu passe -1 en argument au write(), casté
automatiquement en "size_t" qui est un type non signé. C'est donc une
demande d'écriture de typiquement pas loin de 4 Go qui est formulée.
Suivant les OS, tu te prendras :
- Une segfault : dépassement du buffer en dehors de la zone
réservée ;
- Un remplissage du disque dur, suivant la politique d'attribution
de la mémoire par l'OS ;
- Une boucle infinie : read() renvoie -1, le noyau détecte que
le write() demande n'importe quoi et renvoie -1, la boucle
recommence...
Tout cela est peu élégant.
- Il est concevable que write écrive strictement moins que howmany tout en
réussissant (penser à une écriture sur une disquette montée en
synchrone, interrompue par un signal).
Portabilité et spécificités Unix
- La partie avec mmap() ne marche que si le fichier est entièrement
mmapable. Ça ne risque pas de marcher avec les fichiers dont on parle
dans cette discussion (fichier de pas loin de 4 Go, voire plus, sur un
PC).
- Dans le mmap(), tu supposes que la sortie est lseek()able. Ce n'est
pas le cas si tu copies à destination d'un fifo nommé.
- Le double mmap() est lourdingue et inutile : il te suffirait de
faire un mmap() de l'entrée, et un write() vers la sortie. Ça donnerait
de meilleures chances d'optimisation à l'OS, et ça économiserait
les mappings mémoire. Ça éviterait aussi le write() d'un caractère
("x") qui te sert à fixer la taille du fichier.
- Si le disque devient plein, tu pourras avoir un échec à l'intérieur
du memcpy(). Ça veut dire un signal (SIGBUS ou SIGSEGV, probablement),
d'où une mort abrupte du processus.
- D'ailleurs, Single Unix ne spécifie pas précisément quand sont
propagées les modifications en écriture via mmap(). Un usage courant
est de le faire au plus tard lors du munmap(), mais rien ne l'impose à
ma connaissance. La façon portable est d'utiliser msync() (avec le flag
MS_ASYNC si on veut juste rendre le nouveau contenu du fichier visible
depuis les autres processus ; MS_SYNC fait un boulot équivalent à
fsync(), ce qui est un overkill coûteux dans la plupart des cas).
- Si l'entrée ou la sortie n'est pas mmap()able (ça peut arriver avec
des fifos, ou avec certains filesystems), tu sors immédiatement plutôt
que de revenir à la méthode avec buffer.
- Tu n'as pas de O_EXCL sur ton fichier en écriture.
Ça donne quoi si c'est un pipe nommé ? Tu vas le détruire, ou écrire
dedans ? Dans le 2e cas, tes write ont des chances de faire quelque
chose de drôle...
- Côté taille de tampon, on peut faire un fstat sur les deux
fichiers après ouverture, et essayer de prendre le max des st_blksize.
Quelque part, c'est un peu fait pour.
- Les droits d'accès du fichier créé sont arbitraires (0644). Tu
devrais recopier ceux du fichier d'origine, ou alors utiliser 0666 et
laisser l'umask faire son boulot. Sur certains OS (notamment certains
distributions Linux), l'umask naturel est 002 et ton 0644 met la zone.
Et je ne parle pas de si je copie
un exécutable.