Impression de me compliquer ... (strcat/sprintf)

Kinou

14/11/2004 à 20:30

Charlie Gordon wrote:

Je ne comprends pas la logique des BUFFER_SIZE ici.
Ni la soustraction nbBuffer-1
Et il manque au moins un char pour le '' final : il faut len+1.
Enfin en C sizeof(char) vaut 1 par definition.

Ces lignes sont en fait dans un while sur un fgets
while(fgets(buffer,sizeof(buffer),fp)) {

BUFFER_SIZE etant donc la taille du buffer utilisé. nbBuffer étant le
numero du buffer actuellement traité (pour savoir si on a du de nouveau
faire un malloc car la taille etait trop petite).

tempLine[0]='';

Cette ligne est inutile, la destination de strcpy() n'a pas besoin d'être
initialisée.

Ok, merci.

Oui. Mais l'emploi du passé correspond uniquement à l'ordre dans lequel tu fais
l'apprentissage de ces langages, pas l'ordre historique de leur conception. C
est antérieur à PHP, Perl, C++, java... Seules des versions primitives et
obsolètes de Basic le précèdent. Donc c'est plutot un ancêtre encore
relativement vert, et dont on respecte avec humilité les limitations.

En effet, je parlais de mon ordre d'apprentissage, le JAVA venant après
le C.
Je crois que si l'on ne respecte pas les limitations de C et que l'on
est toujours à pester contre elles, il faut passer à un autre langage ;)

Chqrlie.

--
Merci,
Vincent

Charlie Gordon

15/11/2004 à 00:01

"Kinou" wrote in message
news:4197b268$0$2411$

Charlie Gordon wrote:

Je ne comprends pas la logique des BUFFER_SIZE ici.
Ni la soustraction nbBuffer-1
Et il manque au moins un char pour le '' final : il faut len+1.
Enfin en C sizeof(char) vaut 1 par definition.

Ces lignes sont en fait dans un while sur un fgets
while(fgets(buffer,sizeof(buffer),fp)) {

BUFFER_SIZE etant donc la taille du buffer utilisé. nbBuffer étant le
numero du buffer actuellement traité (pour savoir si on a du de nouveau
faire un malloc car la taille etait trop petite).

Ah, je viens de comprendre.
C'est la suite de ta file "Travailler sur de gros fichiers".
J'ai relu ton code de readByLine().
Cette fonction lit une ligne complete, éventuellement très longue, et en renvoie
une copie allouée par malloc().
Il y avait dans ton code plusieurs problemes :
- tu testes buffer[len-1] sans vérifier que len est > 0.
on peut supposer que len n'est jamais nul, mais ce n'est pas prudent.
- tu ne testes pas le résultat de l'allocation mémoire.
- la gestion de la taille déjà lue est trop lourde parce que tu utilises un
compte de buffers plutot qu'un compte de caractères.
- tu mélanges l'anglais et le francais dans les noms de variables, c'est confus.

Voici une version simplifiee :

/* Cette fonction lit une ligne complete et retourne
* une copie allouée par malloc()
*/

char *readOneLine(FILE *fp)
{
char buffer[BUFFER_SIZE];
char *line, *newLine;
int len, done, lineLength;

line = NULL;
lineLength = 0;
done = 0;

while (!done && fgets(buffer, sizeof(buffer), fp)) {

len = strlen(buffer);
// Nous sommes a la fin de la line
if (len > 0 && buffer[len - 1] == 'n') {
buffer[len - 1] = '';
len--;
done = 1;
}

newLine = (char *)malloc(lineLength + len + 1);
if (!newLine) {
// Allocation memoire impossible, il faudrait sans
// doute signaler ce probleme.
free(line);
return NULL;
}

if (line) {
// recopie de la partie déjà lue, sauf la premiere fois
// en fait ce test est inutile, mais il rend le code
// plus comprehensible.
memcpy(newLine, line, lineLength);
free(line);
line = NULL;
}

// recopie du dernier buffer lu en fin de bloc
// on pourrait faire un peu plus rapide avec memcpy
strcpy(newLine + lineLength, buffer);
lineLength += len;

line = newLine;
newLine = NULL;
}

// En fin de fichier ou en fin de line, on renvoie la partie
// deja lue. s'il n'y avait plus rien à lire, line==NULL

return line;
}

On pourrait faire un peu plus simple et plus efficace avec realloc(), mais le
mecanisme est
plus clair en faisant l'allocation a la main, et realloc() est un peu trop
piegeuse pour un débutant ;-)

--
Chqrlie.

"Kinou" <kinou@animezvous.com> wrote in message
news:4197b268$0$2411$626a14ce@news.free.fr...

Charlie Gordon wrote:

Je ne comprends pas la logique des BUFFER_SIZE ici.
Ni la soustraction nbBuffer-1
Et il manque au moins un char pour le '' final : il faut len+1.
Enfin en C sizeof(char) vaut 1 par definition.

Ces lignes sont en fait dans un while sur un fgets
while(fgets(buffer,sizeof(buffer),fp)) {

BUFFER_SIZE etant donc la taille du buffer utilisé. nbBuffer étant le
numero du buffer actuellement traité (pour savoir si on a du de nouveau
faire un malloc car la taille etait trop petite).

Ah, je viens de comprendre.
C'est la suite de ta file "Travailler sur de gros fichiers".
J'ai relu ton code de readByLine().
Cette fonction lit une ligne complete, éventuellement très longue, et en renvoie
une copie allouée par malloc().
Il y avait dans ton code plusieurs problemes :
- tu testes buffer[len-1] sans vérifier que len est > 0.
on peut supposer que len n'est jamais nul, mais ce n'est pas prudent.
- tu ne testes pas le résultat de l'allocation mémoire.
- la gestion de la taille déjà lue est trop lourde parce que tu utilises un
compte de buffers plutot qu'un compte de caractères.
- tu mélanges l'anglais et le francais dans les noms de variables, c'est confus.

Voici une version simplifiee :

/* Cette fonction lit une ligne complete et retourne
* une copie allouée par malloc()
*/

char *readOneLine(FILE *fp)
{
char buffer[BUFFER_SIZE];
char *line, *newLine;
int len, done, lineLength;

line = NULL;
lineLength = 0;
done = 0;

while (!done && fgets(buffer, sizeof(buffer), fp)) {

len = strlen(buffer);
// Nous sommes a la fin de la line
if (len > 0 && buffer[len - 1] == 'n') {
buffer[len - 1] = '';
len--;
done = 1;
}

newLine = (char *)malloc(lineLength + len + 1);
if (!newLine) {
// Allocation memoire impossible, il faudrait sans
// doute signaler ce probleme.
free(line);
return NULL;
}

if (line) {
// recopie de la partie déjà lue, sauf la premiere fois
// en fait ce test est inutile, mais il rend le code
// plus comprehensible.
memcpy(newLine, line, lineLength);
free(line);
line = NULL;
}

// recopie du dernier buffer lu en fin de bloc
// on pourrait faire un peu plus rapide avec memcpy
strcpy(newLine + lineLength, buffer);
lineLength += len;

line = newLine;
newLine = NULL;
}

// En fin de fichier ou en fin de line, on renvoie la partie
// deja lue. s'il n'y avait plus rien à lire, line==NULL

return line;
}

On pourrait faire un peu plus simple et plus efficace avec realloc(), mais le
mecanisme est
plus clair en faisant l'allocation a la main, et realloc() est un peu trop
piegeuse pour un débutant ;-)

--
Chqrlie.

Vous avez filtré cet utilisateur ! Consultez son message

"Kinou" wrote in message
news:4197b268$0$2411$

Charlie Gordon wrote:

Je ne comprends pas la logique des BUFFER_SIZE ici.
Ni la soustraction nbBuffer-1
Et il manque au moins un char pour le '' final : il faut len+1.
Enfin en C sizeof(char) vaut 1 par definition.

Ces lignes sont en fait dans un while sur un fgets
while(fgets(buffer,sizeof(buffer),fp)) {

BUFFER_SIZE etant donc la taille du buffer utilisé. nbBuffer étant le
numero du buffer actuellement traité (pour savoir si on a du de nouveau
faire un malloc car la taille etait trop petite).

Ah, je viens de comprendre.
C'est la suite de ta file "Travailler sur de gros fichiers".
J'ai relu ton code de readByLine().
Cette fonction lit une ligne complete, éventuellement très longue, et en renvoie
une copie allouée par malloc().
Il y avait dans ton code plusieurs problemes :
- tu testes buffer[len-1] sans vérifier que len est > 0.
on peut supposer que len n'est jamais nul, mais ce n'est pas prudent.
- tu ne testes pas le résultat de l'allocation mémoire.
- la gestion de la taille déjà lue est trop lourde parce que tu utilises un
compte de buffers plutot qu'un compte de caractères.
- tu mélanges l'anglais et le francais dans les noms de variables, c'est confus.

Voici une version simplifiee :

/* Cette fonction lit une ligne complete et retourne
* une copie allouée par malloc()
*/

char *readOneLine(FILE *fp)
{
char buffer[BUFFER_SIZE];
char *line, *newLine;
int len, done, lineLength;

line = NULL;
lineLength = 0;
done = 0;

while (!done && fgets(buffer, sizeof(buffer), fp)) {

len = strlen(buffer);
// Nous sommes a la fin de la line
if (len > 0 && buffer[len - 1] == 'n') {
buffer[len - 1] = '';
len--;
done = 1;
}

newLine = (char *)malloc(lineLength + len + 1);
if (!newLine) {
// Allocation memoire impossible, il faudrait sans
// doute signaler ce probleme.
free(line);
return NULL;
}

if (line) {
// recopie de la partie déjà lue, sauf la premiere fois
// en fait ce test est inutile, mais il rend le code
// plus comprehensible.
memcpy(newLine, line, lineLength);
free(line);
line = NULL;
}

// recopie du dernier buffer lu en fin de bloc
// on pourrait faire un peu plus rapide avec memcpy
strcpy(newLine + lineLength, buffer);
lineLength += len;

line = newLine;
newLine = NULL;
}

// En fin de fichier ou en fin de line, on renvoie la partie
// deja lue. s'il n'y avait plus rien à lire, line==NULL

return line;
}

On pourrait faire un peu plus simple et plus efficace avec realloc(), mais le
mecanisme est
plus clair en faisant l'allocation a la main, et realloc() est un peu trop
piegeuse pour un débutant ;-)

--
Chqrlie.

rtroadec

15/11/2004 à 18:28

"Charlie Gordon" wrote in message news:<cn6ho9$5m$...

Salut,

Exple (append a faire en fonction du parsing):

#include <stdio.h>

char * appendChar(char * s, size_t lng, char c)
{
if (s != NULL)
{
size_t pos = strlen(s);
if (pos < lng-1)

les unsigned ont encore frappé !
ce test se vautre si lng vaut 0
il faut mieux écrire pos + 1 < lng
ou utiliser des ssize_t ou des int pour les tailles de tableau.
qui plus est lng est mal choisi comme nom pour la taille du tableau, c'est trop
ambigu, on pourrait confondre avec la longueur de la chaine qui s'y trouve.

Aïe, bien vu, je me suis dit aujourd'hui "Tiens, je pourrais corriger
par if (pos < (int)(lng-1))", et là, erreur fatale (cause priorité
opérateurs), ça revient au même et c'est pire, ca obfusque davantage.
if (pos < (int)lng-1) serait correct avec toutefois un avertissement
sur la comparaison signé/non signé.

{
s[pos++] = c;
s[pos] = '';
}
}
return s;
}

[coupé]

#include <stdio.h>
#include <stdlib.h>

int main(void)
{
char * dst_s, * dst_s_sav;
char * part1_s = "Il y a une ";
char * part2_s = "greve a l'aeroport de Brest et ca m'emmerde beaucoup";

/* Allocation dst_s de 20 (chaine de 19 car. max) */
dst_s = calloc(20, sizeof*dst_s);
if (dst_s)
{
/* On popule une premiere fois dst_s*/
strncat(dst_s, part1_s, 19);
/* On affiche */
puts("CHAINE ORIGINALE");
puts(dst_s);
/* On souhaite un peu de place en plus */
/* On passe a une taille de 50, soit 49 car. max */
/* On utilise pour cela une variable pointeur temporaire*/
dst_s_sav = realloc(dst_s, 50);

quel drole de nom pour cette variable temporaire : dst_s_sav.

dst_s_tmp si tu préfères.

if (dst_s_sav)
{
size_t lng;
/* Reallocation reussie, on recupere le nouveau bloc */
dst_s = dst_s_sav;
/* ATTENTION, ne pas oublier de retrancher le nombre de
caractères effectivement ecrits dans le buffer original */
lng = (strlen(part1_s)<19) ? strlen(part1_s) : 19;

En fait c'est tout simplement : lng = strlen(dst_s);

Oui, ici c'était à titre d'illustatrion dans le cadre de
l'agradissement de bloc.

strncat(dst_s, part2_s, 50-lng);

pas de bol, c'est 49-lng. Et encore on suppose que lng < 49, sinon ca fera un
debordement de tableau.
strncat() est un faux ami.

Aïe, mea culpa, surtout après le gros thread des jours derniers sur
strncpy/strncat et les nombreuse propositions. Mais je reste sur mon
opinion concernant strncat, le fait est simplement que je ne le
maitrîse pas encore. Pour lng < 49, pas de pb, ici j'avais mis part1_s
en dur mais je fais plus de contrôles dans la vraie vie ;) quand je
code.

Regis

"Charlie Gordon" <news@chqrlie.org> wrote in message news:<cn6ho9$5m$1@reader1.imaginet.fr>...

Salut,

Exple (append a faire en fonction du parsing):

#include <stdio.h>

char * appendChar(char * s, size_t lng, char c)
{
if (s != NULL)
{
size_t pos = strlen(s);
if (pos < lng-1)

les unsigned ont encore frappé !
ce test se vautre si lng vaut 0
il faut mieux écrire pos + 1 < lng
ou utiliser des ssize_t ou des int pour les tailles de tableau.
qui plus est lng est mal choisi comme nom pour la taille du tableau, c'est trop
ambigu, on pourrait confondre avec la longueur de la chaine qui s'y trouve.

Aïe, bien vu, je me suis dit aujourd'hui "Tiens, je pourrais corriger
par if (pos < (int)(lng-1))", et là, erreur fatale (cause priorité
opérateurs), ça revient au même et c'est pire, ca obfusque davantage.
if (pos < (int)lng-1) serait correct avec toutefois un avertissement
sur la comparaison signé/non signé.

{
s[pos++] = c;
s[pos] = '';
}
}
return s;
}

[coupé]

#include <stdio.h>
#include <stdlib.h>

int main(void)
{
char * dst_s, * dst_s_sav;
char * part1_s = "Il y a une ";
char * part2_s = "greve a l'aeroport de Brest et ca m'emmerde beaucoup";

/* Allocation dst_s de 20 (chaine de 19 car. max) */
dst_s = calloc(20, sizeof*dst_s);
if (dst_s)
{
/* On popule une premiere fois dst_s*/
strncat(dst_s, part1_s, 19);
/* On affiche */
puts("CHAINE ORIGINALE");
puts(dst_s);
/* On souhaite un peu de place en plus */
/* On passe a une taille de 50, soit 49 car. max */
/* On utilise pour cela une variable pointeur temporaire*/
dst_s_sav = realloc(dst_s, 50);

quel drole de nom pour cette variable temporaire : dst_s_sav.

dst_s_tmp si tu préfères.

if (dst_s_sav)
{
size_t lng;
/* Reallocation reussie, on recupere le nouveau bloc */
dst_s = dst_s_sav;
/* ATTENTION, ne pas oublier de retrancher le nombre de
caractères effectivement ecrits dans le buffer original */
lng = (strlen(part1_s)<19) ? strlen(part1_s) : 19;

En fait c'est tout simplement : lng = strlen(dst_s);

Oui, ici c'était à titre d'illustatrion dans le cadre de
l'agradissement de bloc.

strncat(dst_s, part2_s, 50-lng);

pas de bol, c'est 49-lng. Et encore on suppose que lng < 49, sinon ca fera un
debordement de tableau.
strncat() est un faux ami.

Aïe, mea culpa, surtout après le gros thread des jours derniers sur
strncpy/strncat et les nombreuse propositions. Mais je reste sur mon
opinion concernant strncat, le fait est simplement que je ne le
maitrîse pas encore. Pour lng < 49, pas de pb, ici j'avais mis part1_s
en dur mais je fais plus de contrôles dans la vraie vie ;) quand je
code.

Regis

Vous avez filtré cet utilisateur ! Consultez son message

"Charlie Gordon" wrote in message news:<cn6ho9$5m$...

Salut,

Exple (append a faire en fonction du parsing):

#include <stdio.h>

char * appendChar(char * s, size_t lng, char c)
{
if (s != NULL)
{
size_t pos = strlen(s);
if (pos < lng-1)

les unsigned ont encore frappé !
ce test se vautre si lng vaut 0
il faut mieux écrire pos + 1 < lng
ou utiliser des ssize_t ou des int pour les tailles de tableau.
qui plus est lng est mal choisi comme nom pour la taille du tableau, c'est trop
ambigu, on pourrait confondre avec la longueur de la chaine qui s'y trouve.

Aïe, bien vu, je me suis dit aujourd'hui "Tiens, je pourrais corriger
par if (pos < (int)(lng-1))", et là, erreur fatale (cause priorité
opérateurs), ça revient au même et c'est pire, ca obfusque davantage.
if (pos < (int)lng-1) serait correct avec toutefois un avertissement
sur la comparaison signé/non signé.

{
s[pos++] = c;
s[pos] = '';
}
}
return s;
}

[coupé]

#include <stdio.h>
#include <stdlib.h>

int main(void)
{
char * dst_s, * dst_s_sav;
char * part1_s = "Il y a une ";
char * part2_s = "greve a l'aeroport de Brest et ca m'emmerde beaucoup";

/* Allocation dst_s de 20 (chaine de 19 car. max) */
dst_s = calloc(20, sizeof*dst_s);
if (dst_s)
{
/* On popule une premiere fois dst_s*/
strncat(dst_s, part1_s, 19);
/* On affiche */
puts("CHAINE ORIGINALE");
puts(dst_s);
/* On souhaite un peu de place en plus */
/* On passe a une taille de 50, soit 49 car. max */
/* On utilise pour cela une variable pointeur temporaire*/
dst_s_sav = realloc(dst_s, 50);

quel drole de nom pour cette variable temporaire : dst_s_sav.

dst_s_tmp si tu préfères.

if (dst_s_sav)
{
size_t lng;
/* Reallocation reussie, on recupere le nouveau bloc */
dst_s = dst_s_sav;
/* ATTENTION, ne pas oublier de retrancher le nombre de
caractères effectivement ecrits dans le buffer original */
lng = (strlen(part1_s)<19) ? strlen(part1_s) : 19;

En fait c'est tout simplement : lng = strlen(dst_s);

Oui, ici c'était à titre d'illustatrion dans le cadre de
l'agradissement de bloc.

strncat(dst_s, part2_s, 50-lng);

pas de bol, c'est 49-lng. Et encore on suppose que lng < 49, sinon ca fera un
debordement de tableau.
strncat() est un faux ami.

Aïe, mea culpa, surtout après le gros thread des jours derniers sur
strncpy/strncat et les nombreuse propositions. Mais je reste sur mon
opinion concernant strncat, le fait est simplement que je ne le
maitrîse pas encore. Pour lng < 49, pas de pb, ici j'avais mis part1_s
en dur mais je fais plus de contrôles dans la vraie vie ;) quand je
code.

Regis

Kinou

15/11/2004 à 18:32

Charlie Gordon wrote:

"Kinou" wrote in message
news:4197b268$0$2411$

Ah, je viens de comprendre.
C'est la suite de ta file "Travailler sur de gros fichiers".

Ce n'est pas exactement la suite, mais un probleme similaire qui se
posait autre part.

J'ai relu ton code de readByLine().
Cette fonction lit une ligne complete, éventuellement très longue, et en renvoie
une copie allouée par malloc().
Il y avait dans ton code plusieurs problemes :
- tu testes buffer[len-1] sans vérifier que len est > 0.
on peut supposer que len n'est jamais nul, mais ce n'est pas prudent.

Certes, ce n'est pas tres prudent comme tu le dis.

- tu ne testes pas le résultat de l'allocation mémoire.

Oui je ne le faisais pas au début, j'ai corrigé cela.

- la gestion de la taille déjà lue est trop lourde parce que tu utilises un
compte de buffers plutot qu'un compte de caractères.
- tu mélanges l'anglais et le francais dans les noms de variables, c'est confus.

Voici une version simplifiee :

/* Cette fonction lit une ligne complete et retourne
* une copie allouée par malloc()
*/

char *readOneLine(FILE *fp)
{
char buffer[BUFFER_SIZE];
char *line, *newLine;
int len, done, lineLength;

line = NULL;
lineLength = 0;
done = 0;

while (!done && fgets(buffer, sizeof(buffer), fp)) {

len = strlen(buffer);
// Nous sommes a la fin de la line
if (len > 0 && buffer[len - 1] == 'n') {
buffer[len - 1] = '';
len--;
done = 1;
}

newLine = (char *)malloc(lineLength + len + 1);
if (!newLine) {
// Allocation memoire impossible, il faudrait sans
// doute signaler ce probleme.
free(line);
return NULL;
}

if (line) {
// recopie de la partie déjà lue, sauf la premiere fois
// en fait ce test est inutile, mais il rend le code
// plus comprehensible.
memcpy(newLine, line, lineLength);
free(line);
line = NULL;
}

// recopie du dernier buffer lu en fin de bloc
// on pourrait faire un peu plus rapide avec memcpy
strcpy(newLine + lineLength, buffer);
lineLength += len;

line = newLine;
newLine = NULL;
}

// En fin de fichier ou en fin de line, on renvoie la partie
// deja lue. s'il n'y avait plus rien à lire, line==NULL

return line;
}

Merci ! memcpy est plus efficace que strcpy ?

On pourrait faire un peu plus simple et plus efficace avec realloc(), mais le
mecanisme est
plus clair en faisant l'allocation a la main, et realloc() est un peu trop
piegeuse pour un débutant ;-)

Yep si il y a quelquechose que j'ai compris, c'est ca ;-)

--
Chqrlie.

--
Cordialement,
Vincent

Charlie Gordon wrote:

"Kinou" <kinou@animezvous.com> wrote in message
news:4197b268$0$2411$626a14ce@news.free.fr...

Ah, je viens de comprendre.
C'est la suite de ta file "Travailler sur de gros fichiers".

Ce n'est pas exactement la suite, mais un probleme similaire qui se
posait autre part.

J'ai relu ton code de readByLine().
Cette fonction lit une ligne complete, éventuellement très longue, et en renvoie
une copie allouée par malloc().
Il y avait dans ton code plusieurs problemes :
- tu testes buffer[len-1] sans vérifier que len est > 0.
on peut supposer que len n'est jamais nul, mais ce n'est pas prudent.

Certes, ce n'est pas tres prudent comme tu le dis.

- tu ne testes pas le résultat de l'allocation mémoire.

Oui je ne le faisais pas au début, j'ai corrigé cela.

- la gestion de la taille déjà lue est trop lourde parce que tu utilises un
compte de buffers plutot qu'un compte de caractères.
- tu mélanges l'anglais et le francais dans les noms de variables, c'est confus.

Voici une version simplifiee :

/* Cette fonction lit une ligne complete et retourne
* une copie allouée par malloc()
*/

char *readOneLine(FILE *fp)
{
char buffer[BUFFER_SIZE];
char *line, *newLine;
int len, done, lineLength;

line = NULL;
lineLength = 0;
done = 0;

while (!done && fgets(buffer, sizeof(buffer), fp)) {

len = strlen(buffer);
// Nous sommes a la fin de la line
if (len > 0 && buffer[len - 1] == 'n') {
buffer[len - 1] = '';
len--;
done = 1;
}

newLine = (char *)malloc(lineLength + len + 1);
if (!newLine) {
// Allocation memoire impossible, il faudrait sans
// doute signaler ce probleme.
free(line);
return NULL;
}

if (line) {
// recopie de la partie déjà lue, sauf la premiere fois
// en fait ce test est inutile, mais il rend le code
// plus comprehensible.
memcpy(newLine, line, lineLength);
free(line);
line = NULL;
}

// recopie du dernier buffer lu en fin de bloc
// on pourrait faire un peu plus rapide avec memcpy
strcpy(newLine + lineLength, buffer);
lineLength += len;

line = newLine;
newLine = NULL;
}

// En fin de fichier ou en fin de line, on renvoie la partie
// deja lue. s'il n'y avait plus rien à lire, line==NULL

return line;
}

Merci ! memcpy est plus efficace que strcpy ?

On pourrait faire un peu plus simple et plus efficace avec realloc(), mais le
mecanisme est
plus clair en faisant l'allocation a la main, et realloc() est un peu trop
piegeuse pour un débutant ;-)

Yep si il y a quelquechose que j'ai compris, c'est ca ;-)

--
Chqrlie.

--
Cordialement,
Vincent

Vous avez filtré cet utilisateur ! Consultez son message

Charlie Gordon wrote:

"Kinou" wrote in message
news:4197b268$0$2411$

Ah, je viens de comprendre.
C'est la suite de ta file "Travailler sur de gros fichiers".

Ce n'est pas exactement la suite, mais un probleme similaire qui se
posait autre part.

J'ai relu ton code de readByLine().
Cette fonction lit une ligne complete, éventuellement très longue, et en renvoie
une copie allouée par malloc().
Il y avait dans ton code plusieurs problemes :
- tu testes buffer[len-1] sans vérifier que len est > 0.
on peut supposer que len n'est jamais nul, mais ce n'est pas prudent.

Certes, ce n'est pas tres prudent comme tu le dis.

- tu ne testes pas le résultat de l'allocation mémoire.

Oui je ne le faisais pas au début, j'ai corrigé cela.

- la gestion de la taille déjà lue est trop lourde parce que tu utilises un
compte de buffers plutot qu'un compte de caractères.
- tu mélanges l'anglais et le francais dans les noms de variables, c'est confus.

Voici une version simplifiee :

/* Cette fonction lit une ligne complete et retourne
* une copie allouée par malloc()
*/

char *readOneLine(FILE *fp)
{
char buffer[BUFFER_SIZE];
char *line, *newLine;
int len, done, lineLength;

line = NULL;
lineLength = 0;
done = 0;

while (!done && fgets(buffer, sizeof(buffer), fp)) {

len = strlen(buffer);
// Nous sommes a la fin de la line
if (len > 0 && buffer[len - 1] == 'n') {
buffer[len - 1] = '';
len--;
done = 1;
}

newLine = (char *)malloc(lineLength + len + 1);
if (!newLine) {
// Allocation memoire impossible, il faudrait sans
// doute signaler ce probleme.
free(line);
return NULL;
}

if (line) {
// recopie de la partie déjà lue, sauf la premiere fois
// en fait ce test est inutile, mais il rend le code
// plus comprehensible.
memcpy(newLine, line, lineLength);
free(line);
line = NULL;
}

// recopie du dernier buffer lu en fin de bloc
// on pourrait faire un peu plus rapide avec memcpy
strcpy(newLine + lineLength, buffer);
lineLength += len;

line = newLine;
newLine = NULL;
}

// En fin de fichier ou en fin de line, on renvoie la partie
// deja lue. s'il n'y avait plus rien à lire, line==NULL

return line;
}

Merci ! memcpy est plus efficace que strcpy ?

On pourrait faire un peu plus simple et plus efficace avec realloc(), mais le
mecanisme est
plus clair en faisant l'allocation a la main, et realloc() est un peu trop
piegeuse pour un débutant ;-)

Yep si il y a quelquechose que j'ai compris, c'est ca ;-)

--
Chqrlie.

--
Cordialement,
Vincent

Charlie Gordon

15/11/2004 à 18:51

Merci ! memcpy est plus efficace que strcpy ?

Oui, memcpy est plus efficace que strcpy : en règle générale, strcpy doit tester
la valeur de chaque caractère de la chaine source pour determiner quand
s'arreter de copier, alors que memcpy en connait le nombre dès le départ, voire
même dès la compilation.
La librairie standard glibc optimise memcpy a mort, remplaçant la plupart des
appels par du code généré directement en lieu et place de l'appel, et
particulierement efficace quant la taille de la copie est constante et
l'alignement des pointeurs connu.
Strcpy est également optimisée à la compilation, en particulier dans le cas ou
la chaine source est constante (c-a-d littérale). Le code généré dans ce cas
est très similaire à celui de memcpy :

strcpy(dest, "une chaine"); est équivalent à
memcpy(dest, "une chaine", sizeof("une chaine"));

Tu peux tenter d'analyser le contenu du header string.h de la lib C. C'est
particulièrement instructif.

Chqrlie.

Antoine Leca

15/11/2004 à 18:51

En , Targeur fou va escriure:

"Charlie Gordon" wrote in message
news:<cn6ho9$5m$...

char * appendChar(char * s, size_t lng, char c) {
size_t pos = strlen(s);
if (pos < lng-1)

les unsigned ont encore frappé !
ce test se vautre si lng vaut 0

Aïe, bien vu, je me suis dit aujourd'hui "Tiens, je pourrais corriger
par if (pos < (int)(lng-1))", et là, erreur fatale (cause priorité
opérateurs), ça revient au même et c'est pire, ca obfusque davantage.

Oui.

if (pos < (int)lng-1) serait correct

Ah?

avec toutefois un avertissement sur la comparaison signé/non signé.

... et hé ... Zorro est <coupé>
Voyons dans le détail:
lng est un size_t, transformé sans autre forme de procès en int. Admettons
que ce soit sans souci (cas général). Appelons donc lng_i le résultat (un
int).

lng_i - 1 est la différence entre deux int, cela reste un int
(éventuellement égal à -1 dans le cas limite qui nous préoccupe).

De l'autre côté, nous avons pos. Autre size_t, donc très très probablement
un type non signé (obligatoire si ton compilateur respecte la norme, en
fait). Et selon toute probabilité, ce ne sera ni unsigned short, ni unsigned
char, mais plutôt unsigned int ou quelque chose de « plus grand ».

Maintenant, la pregunta del millón: un entier non signé, de taille «
supérieure ou égale » à unsigned int, quand on le compare avec un int, que
se passe-t'il ? Qui gagne, à ton avis ?

Antoine

Emmanuel Delahaye

15/11/2004 à 19:27

Kinou wrote on 15/11/04 :

Merci ! memcpy est plus efficace que strcpy ?

Oui, si on connait la longueur à copier. Sinon, on doit utiliser
strlen(), et on tombe dans le même travers que strcpy() (temps qui
s'allonge avec la taille de la chaine).

--
Emmanuel
The C-FAQ: http://www.eskimo.com/~scs/C-faq/faq.html
The C-library: http://www.dinkumware.com/refxc.html

"C is a sharp tool"

Charlie Gordon

15/11/2004 à 19:38

"Targeur fou" wrote in message
news:

"Charlie Gordon" wrote in message
news:<cn6ho9$5m$...

size_t pos = strlen(s);
if (pos < lng-1)

les unsigned ont encore frappé !
ce test se vautre si lng vaut 0
il faut mieux écrire pos + 1 < lng
ou utiliser des ssize_t ou des int pour les tailles de tableau.
qui plus est lng est mal choisi comme nom pour la taille du tableau, c'est
trop

ambigu, on pourrait confondre avec la longueur de la chaine qui s'y trouve.

Aïe, bien vu, je me suis dit aujourd'hui "Tiens, je pourrais corriger
par if (pos < (int)(lng-1))", et là, erreur fatale (cause priorité
opérateurs), ça revient au même et c'est pire, ca obfusque davantage.
if (pos < (int)lng-1) serait correct avec toutefois un avertissement
sur la comparaison signé/non signé.

Hélas, non, cela ne suffirait pas, car la réalité est encore plus sordide :

La comparaison signé/non signé a une sémantique assez piégeuse que tu ne sembles
pas encore maitriser.
Le problème ici n'est pas la précédence des opérations, mais les règles
d'arithmétique non-signée.

pos et lng sont tous deux unsigned
lng-1 cause un overflow si lng est 0, mais le résultat reste unsigned.
(plus surprenant encore : -lng est unsigned aussi, et cause un overflow si lng
est non 0)
la comparaison pos < lng-1 est donc une opération non-signée.
en complément à 2, lng-1 vaut UINT_MAX, donc la comparaison sera vraie alors que
la destination est notoirement trop petite.
(en fait la comparaison serait fausse si la chaine faisait UINT_MAX caractères,
cas que ma proposition n'est pas capable de gérer correctement non plus, mais
qui nécessiterait que le buffer ait une taille encore plus grande, ce qui n'est
pas possible.

Le fait de convertir explicitement lng ou (lng-1) en int ne résout pas le
problème.
La comparaison d'un int et d'un size_t est faite en *unsigned* (avec au mieux un
warning de compilation) et donc donne le même comportement que ci-dessus.

Convertir lng (et non (lng-1)) en long changerait effectivement le sens de la
comparaison, mais uniquement sur les architectures ou les long sont plus grands
que les int, et à condition qu'ils soient aussi plus grands que les size_t. Ce
serait le cas des modèles 16 bits obsolètes de MS/DOS, mais pas nécessairement
des architectures récentes 64 bits, où on peut avoir des long de 64 bits et des
int de 32, mais où les size_t peuvent aussi faire 64 bits. La comparaison se
fait alors encore en non-signé.

Conclusion : cette question est d'une technicité qui force l'humilité. Si le
compilateur emet des warning sur les comparaisons signé/non-signé, il faut
s'assurer que la valeur signée ne peut pas être négative, et d'autre part, il
faut être extrêmement vigilant avec toute soustraction sur des unsigned, qui ne
doit en aucun cas causer d'overflow.

[...]

strncat() est un faux ami.

Aïe, mea culpa, surtout après le gros thread des jours derniers sur
strncpy/strncat et les nombreuse propositions. Mais je reste sur mon
opinion concernant strncat, le fait est simplement que je ne le
maitrîse pas encore. Pour lng < 49, pas de pb, ici j'avais mis part1_s
en dur mais je fais plus de contrôles dans la vraie vie ;) quand je
code.

Je ne partage pas ton optimisme, mais je connais qu'il m'a fallu beaucoup de
temps pour admettre que la question n'est pas de maitriser ou non strncat ou
strncpy, mais de refuser de piéger son propre code avec ces mines qui
attendraient patiemment le programmeur suivant pour lui pourrir la vie avec leur
sémantique ambiguë. La bonne solution est de ne pas utiliser ces saletés, voire
d'en empêcher l'usage, et d'utiliser à la place des fonctions plus claires,
implémentées localement.

Chqrlie.

"Targeur fou" <rtroadec@yahoo.fr> wrote in message
news:99d640fa.0411150928.3364cd11@posting.google.com...

"Charlie Gordon" <news@chqrlie.org> wrote in message
news:<cn6ho9$5m$1@reader1.imaginet.fr>...

size_t pos = strlen(s);
if (pos < lng-1)

les unsigned ont encore frappé !
ce test se vautre si lng vaut 0
il faut mieux écrire pos + 1 < lng
ou utiliser des ssize_t ou des int pour les tailles de tableau.
qui plus est lng est mal choisi comme nom pour la taille du tableau, c'est
trop

ambigu, on pourrait confondre avec la longueur de la chaine qui s'y trouve.

Aïe, bien vu, je me suis dit aujourd'hui "Tiens, je pourrais corriger
par if (pos < (int)(lng-1))", et là, erreur fatale (cause priorité
opérateurs), ça revient au même et c'est pire, ca obfusque davantage.
if (pos < (int)lng-1) serait correct avec toutefois un avertissement
sur la comparaison signé/non signé.

Hélas, non, cela ne suffirait pas, car la réalité est encore plus sordide :

La comparaison signé/non signé a une sémantique assez piégeuse que tu ne sembles
pas encore maitriser.
Le problème ici n'est pas la précédence des opérations, mais les règles
d'arithmétique non-signée.

pos et lng sont tous deux unsigned
lng-1 cause un overflow si lng est 0, mais le résultat reste unsigned.
(plus surprenant encore : -lng est unsigned aussi, et cause un overflow si lng
est non 0)
la comparaison pos < lng-1 est donc une opération non-signée.
en complément à 2, lng-1 vaut UINT_MAX, donc la comparaison sera vraie alors que
la destination est notoirement trop petite.
(en fait la comparaison serait fausse si la chaine faisait UINT_MAX caractères,
cas que ma proposition n'est pas capable de gérer correctement non plus, mais
qui nécessiterait que le buffer ait une taille encore plus grande, ce qui n'est
pas possible.

Le fait de convertir explicitement lng ou (lng-1) en int ne résout pas le
problème.
La comparaison d'un int et d'un size_t est faite en *unsigned* (avec au mieux un
warning de compilation) et donc donne le même comportement que ci-dessus.

Convertir lng (et non (lng-1)) en long changerait effectivement le sens de la
comparaison, mais uniquement sur les architectures ou les long sont plus grands
que les int, et à condition qu'ils soient aussi plus grands que les size_t. Ce
serait le cas des modèles 16 bits obsolètes de MS/DOS, mais pas nécessairement
des architectures récentes 64 bits, où on peut avoir des long de 64 bits et des
int de 32, mais où les size_t peuvent aussi faire 64 bits. La comparaison se
fait alors encore en non-signé.

Conclusion : cette question est d'une technicité qui force l'humilité. Si le
compilateur emet des warning sur les comparaisons signé/non-signé, il faut
s'assurer que la valeur signée ne peut pas être négative, et d'autre part, il
faut être extrêmement vigilant avec toute soustraction sur des unsigned, qui ne
doit en aucun cas causer d'overflow.

[...]

strncat() est un faux ami.

Aïe, mea culpa, surtout après le gros thread des jours derniers sur
strncpy/strncat et les nombreuse propositions. Mais je reste sur mon
opinion concernant strncat, le fait est simplement que je ne le
maitrîse pas encore. Pour lng < 49, pas de pb, ici j'avais mis part1_s
en dur mais je fais plus de contrôles dans la vraie vie ;) quand je
code.

Je ne partage pas ton optimisme, mais je connais qu'il m'a fallu beaucoup de
temps pour admettre que la question n'est pas de maitriser ou non strncat ou
strncpy, mais de refuser de piéger son propre code avec ces mines qui
attendraient patiemment le programmeur suivant pour lui pourrir la vie avec leur
sémantique ambiguë. La bonne solution est de ne pas utiliser ces saletés, voire
d'en empêcher l'usage, et d'utiliser à la place des fonctions plus claires,
implémentées localement.

Chqrlie.

Vous avez filtré cet utilisateur ! Consultez son message

"Targeur fou" wrote in message
news:

"Charlie Gordon" wrote in message
news:<cn6ho9$5m$...

size_t pos = strlen(s);
if (pos < lng-1)

les unsigned ont encore frappé !
ce test se vautre si lng vaut 0
il faut mieux écrire pos + 1 < lng
ou utiliser des ssize_t ou des int pour les tailles de tableau.
qui plus est lng est mal choisi comme nom pour la taille du tableau, c'est
trop

ambigu, on pourrait confondre avec la longueur de la chaine qui s'y trouve.

Aïe, bien vu, je me suis dit aujourd'hui "Tiens, je pourrais corriger
par if (pos < (int)(lng-1))", et là, erreur fatale (cause priorité
opérateurs), ça revient au même et c'est pire, ca obfusque davantage.
if (pos < (int)lng-1) serait correct avec toutefois un avertissement
sur la comparaison signé/non signé.

Hélas, non, cela ne suffirait pas, car la réalité est encore plus sordide :

La comparaison signé/non signé a une sémantique assez piégeuse que tu ne sembles
pas encore maitriser.
Le problème ici n'est pas la précédence des opérations, mais les règles
d'arithmétique non-signée.

pos et lng sont tous deux unsigned
lng-1 cause un overflow si lng est 0, mais le résultat reste unsigned.
(plus surprenant encore : -lng est unsigned aussi, et cause un overflow si lng
est non 0)
la comparaison pos < lng-1 est donc une opération non-signée.
en complément à 2, lng-1 vaut UINT_MAX, donc la comparaison sera vraie alors que
la destination est notoirement trop petite.
(en fait la comparaison serait fausse si la chaine faisait UINT_MAX caractères,
cas que ma proposition n'est pas capable de gérer correctement non plus, mais
qui nécessiterait que le buffer ait une taille encore plus grande, ce qui n'est
pas possible.

Le fait de convertir explicitement lng ou (lng-1) en int ne résout pas le
problème.
La comparaison d'un int et d'un size_t est faite en *unsigned* (avec au mieux un
warning de compilation) et donc donne le même comportement que ci-dessus.

Convertir lng (et non (lng-1)) en long changerait effectivement le sens de la
comparaison, mais uniquement sur les architectures ou les long sont plus grands
que les int, et à condition qu'ils soient aussi plus grands que les size_t. Ce
serait le cas des modèles 16 bits obsolètes de MS/DOS, mais pas nécessairement
des architectures récentes 64 bits, où on peut avoir des long de 64 bits et des
int de 32, mais où les size_t peuvent aussi faire 64 bits. La comparaison se
fait alors encore en non-signé.

Conclusion : cette question est d'une technicité qui force l'humilité. Si le
compilateur emet des warning sur les comparaisons signé/non-signé, il faut
s'assurer que la valeur signée ne peut pas être négative, et d'autre part, il
faut être extrêmement vigilant avec toute soustraction sur des unsigned, qui ne
doit en aucun cas causer d'overflow.

[...]

strncat() est un faux ami.

Aïe, mea culpa, surtout après le gros thread des jours derniers sur
strncpy/strncat et les nombreuse propositions. Mais je reste sur mon
opinion concernant strncat, le fait est simplement que je ne le
maitrîse pas encore. Pour lng < 49, pas de pb, ici j'avais mis part1_s
en dur mais je fais plus de contrôles dans la vraie vie ;) quand je
code.

Je ne partage pas ton optimisme, mais je connais qu'il m'a fallu beaucoup de
temps pour admettre que la question n'est pas de maitriser ou non strncat ou
strncpy, mais de refuser de piéger son propre code avec ces mines qui
attendraient patiemment le programmeur suivant pour lui pourrir la vie avec leur
sémantique ambiguë. La bonne solution est de ne pas utiliser ces saletés, voire
d'en empêcher l'usage, et d'utiliser à la place des fonctions plus claires,
implémentées localement.

Chqrlie.

Charlie Gordon

15/11/2004 à 19:43

"Emmanuel Delahaye" wrote in message
news:

Kinou wrote on 15/11/04 :
Merci ! memcpy est plus efficace que strcpy ?

Oui, si on connait la longueur à copier. Sinon, on doit utiliser
strlen(), et on tombe dans le même travers que strcpy() (temps qui
s'allonge avec la taille de la chaine).

bien sur remplacer les

strcpy(dest, source);

par des

memcpy(dest, source, strlen(source) + 1);

ne gagne rien, et empêche le compilateur ou la libc d'utiliser pour strcpy des
techniques qui pourraient être plus efficaces.

Mais dans le code de Vincent, il se trouve que l'on connait la longueur de
buffer : len qui a ete calculée au début de la boucle, et que j'ai pris soin de
mettre à jour dans le cas du 'n'. Donc on pourrait remplacer

strcpy(newLine + lineLength, buffer);

par

memcpy(newLine + lineLength, buffer, len + 1);

Chqrlie.

Emmanuel Delahaye

15/11/2004 à 19:48

Charlie Gordon wrote on 15/11/04 :

Tu peux tenter d'analyser le contenu du header string.h de la lib C. C'est
particulièrement instructif.

Bof, en dehors de la définition standard, ça dépend totalement de
l'implémentation...

Petite collection glanée sur mon disque...

char * _BUILTIN strcpy(char *s1, char *s2);

char* strcpy (char*, const char*);

#if defined(_OPTIMIZE_FOR_SPACE) && (defined(__TMS470__) ||
defined(__TMS320C2000__))
_OPT_IDECL char *strcpy(char *_dest, const char *_src);
#endif

#if defined(_INLINE) || defined(_STRCPY)
_OPT_IDEFN char *strcpy(register char *dest, register const char *src)
{
register char *d = dest - 1;
register const char *s = src - 1;

while (*++d = *++s);
return dest;
}
#endif /* _INLINE || _STRCPY */

_CRTIMP char * __cdecl strcpy(char *, const char *);
char * __cdecl strcpy(char *, const char *);

#define strcpy _mbscpy

char _FAR * _CType _FARFUNC strcpy(char _FAR *__dest, const char _FAR
*__src);
char _FAR * _RTLENTRYF _EXPFUNC strcpy(char _FAR *__dest, const char
_FAR *__src);

char * strcpy(char *_s1, const char *_s2);

#if __LCCOPTIMLEVEL > 0
char * _stdcall strcpy(char *,const char *);
#else
char * strcpy(char *, const char *);
#endif

Rien de bien instructif à mon sens...

--
Emmanuel
The C-FAQ: http://www.eskimo.com/~scs/C-faq/faq.html
The C-library: http://www.dinkumware.com/refxc.html

"C is a sharp tool"

Impression de me compliquer ... (strcat/sprintf)

10 réponses

Veuillez sélectionner un problème