Twitter iPhone pliant OnePlus 11 PS5 Disney+ Orange Livebox Windows 11

allocation dynamique bis

17 réponses
Avatar
Benoit Izac
Bonjour,

J'ai compliqué un peu la chose en faisant de l'allocation dynamique pour
une entrée dont je ne connais pas la taille par avance et que je ne peux
pas calculer.

Ci-dessous le code, tous commentaires bien venus.
-------------------------------------------------------------------------
#include <errno.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

enum bat_states { UNKNOW, CHARGED, CHARGING, DISCHARGING };
int get_battery_state(int);

#define MALLOC_SIZE 16

int
get_battery_state(int n)
{
int ret = -1;
char *basedir = "/sys/class/power_supply";
char *type = "BAT";
char *basename = "status";
char *fn;
size_t fn_size;
FILE *fp;
int s,
i;
char *nlp; /* newline position */
char *buf; /* buffer for fgets */
size_t buf_size;
char *line; /* line read */
size_t line_size;
char *r; /* return of fgets */

/*
* generate filename
*/
s = snprintf(NULL, 0, "%s/%s%d/%s", basedir, type, n, basename) + 1;
if (s == -1) {
fprintf(stderr, "snprintf(NULL, 0, ...) failed\n");
return -1;
}
fn_size = s;
fn = malloc(fn_size);
if (!fn) {
perror("fn = malloc()");
return -1;
}
s = snprintf(fn, fn_size, "%s/%s%d/%s", basedir, type, n, basename);
if ((s == -1) || ((size_t) s > (fn_size - 1))) {
fprintf(stderr, "snprintf(fn, %d, ...) return %d\n",
(int) fn_size, s);
if (fn)
free(fn);
return -1;
}

/*
* open filename
*/
fp = fopen(fn, "r");
if (!fp) {
perror("fopen() failed");
free(fn);
return -1;
}
free(fn);


/*
* allocate space for buf
*/
buf = malloc(MALLOC_SIZE);
if (!buf) {
perror("buf = malloc()");
fclose(fp);
return -1;
}

/*
* initialisation
*/
line = NULL;
line_size = 0;
i = 0;
if (MALLOC_SIZE < 2) {
fprintf(stderr, "MALLOC_SIZE must be at least 2\n");
fclose(fp);
free(buf);
return -1;
}

/*
* read data
*/
while ((r = fgets(buf, MALLOC_SIZE, fp))) {
nlp = strchr(buf, '\n');
if (nlp)
*nlp = '\0';
/*
* add one byte for the first allocation to include '\0'
*/
line_size += (i == 0 ? 1 : 0);
buf_size = (nlp ? strlen(buf) : (MALLOC_SIZE - 1));
line_size += buf_size;
line = realloc(line, line_size);
if (!line) {
perror("realloc(line)");
fclose(fp);
free(buf);
return -1;
}
if (i == 0) /* line MUST have a '\0' */
*line = '\0';
i++;
strncat(line, buf, buf_size);
if (nlp)
break;
}
free(buf);

if (!r) { /* fgets return NULL */
if (feof(fp))
fprintf(stderr, "fgets receive EOF\n");
else if (ferror(fp))
perror("fgets()");
else
fprintf(stderr, "you should never see this message!");
if (line)
free(line);
fclose(fp);
return -1;
}
fclose(fp);

/*
* compare
*/
if (strcmp(line, "Full") == 0)
ret = CHARGED;
else if (strcmp(line, "Discharging") == 0)
ret = DISCHARGING;
else if (strcmp(line, "Charging") == 0)
ret = CHARGING;
else {
fprintf(stderr, "unknow battery state: %s\n", line);
ret = UNKNOW;
}
free(line);

return ret;
}
-------------------------------------------------------------------------

Petite série de questions au passage :
- comment faites vous pour être sûr que les allocations soient bien
libérées ?
- peut-on remplacer la série if, else if, etc. de strcmp() par un
switch ?
- ce code me parrait bien compliqué pour faire peu de chose, peut-on
le simplifier (tout en gardant la gestion des erreurs) ?

J'ai bien essayé de faire un point de sortie unique de la fonction mais
le code se retrouve inexorablement collé à droite de l'écran (et
j'utilise des indentations de quatre espaces).

--
Benoit Izac

10 réponses

1 2
Avatar
espie
In article , Benoit Izac wrote:
#define MALLOC_SIZE 16


[...]
if (MALLOC_SIZE < 2) {
fprintf(stderr, "MALLOC_SIZE must be at least 2n");
fclose(fp);
free(buf);
return -1;
}



Euh ?

Ca devrait crasher a la compilation, pas au runtime, genre:

#if MALLOC_SIZE < 2
#error "MALLOC_SIZE must be at least 2"
#endif
Avatar
espie
In article , Benoit Izac wrote:
while ((r = fgets(buf, MALLOC_SIZE, fp))) {
nlp = strchr(buf, 'n');
if (nlp)
*nlp = '';
/*
* add one byte for the first allocation to include ''
*/
line_size += (i == 0 ? 1 : 0);
buf_size = (nlp ? strlen(buf) : (MALLOC_SIZE - 1));
line_size += buf_size;
line = realloc(line, line_size);
if (!line) {
perror("realloc(line)");
fclose(fp);
free(buf);
return -1;
}
if (i == 0) /* line MUST have a '' */
*line = '';
i++;
strncat(line, buf, buf_size);
if (nlp)
break;
}



- Utilisation mauvaise de realloc, avec fuites de memoire.
si realloc renvoie NULL, tu ne liberes pas l'ancien tampon (c'est un
classique). Il faut toujours utiliser un idiome du style
temp = realloc(line, line_size);
if (temp) {
line = temp;
} else {
free(line);
...
}

- Relis bien la doc de strncat, on peut arguer que cette fonction ne sert a
rien, en particulier, elle ne fait pas exactement ce que tu crois.

Elle copie effectivement buf vers line, sans depasser de buf_size, mais elle
n'ajoute pas de 0 final si tu arrives au bout.
Avatar
Benoit Izac
Bonjour,

le 15/03/2009 à 12:09, Marc Espie a écrit dans le message
<gpinlv$15jo$ :

- Utilisation mauvaise de realloc, avec fuites de memoire.
si realloc renvoie NULL, tu ne liberes pas l'ancien tampon (c'est un
classique). Il faut toujours utiliser un idiome du style
temp = realloc(line, line_size);
if (temp) {
line = temp;
} else {
free(line);
...
}



Vu.


- Relis bien la doc de strncat, on peut arguer que cette fonction ne sert a
rien, en particulier, elle ne fait pas exactement ce que tu crois.

Elle copie effectivement buf vers line, sans depasser de buf_size, mais elle
n'ajoute pas de 0 final si tu arrives au bout.



? Selon SUSv3 :

| The strncat() function shall append not more than n bytes (a null byte
| and bytes that follow it are not appended) from the array pointed to by
| s2 to the end of the string pointed to by s1. The initial byte of s2
| overwrites the null byte at the end of s1. A terminating null byte is
| always appended to the result. If copying takes place between objects
| that overlap, the behavior is undefined.

--
Benoit Izac
Avatar
-ed-
On 15 mar, 11:52, Benoit Izac wrote:

Petite série de questions au passage :
    - comment faites vous pour être sûr que les allocations soien t bien
    libérées ?



J'utilise un 'traqueur de mémoire personnel'

http://mapage.noos.fr/emdel/clib.htm
module SYSALLOC

mais c'est surtout en respectant les principes de la programmation
structurée (un seul return par fonction, par exemple), que le code est
'naturellement correct'...

    - peut-on remplacer la série if, else if, etc. de strcmp() par un
    switch ?



Non. L'usage général est if-else if. switch-case est limité aux
entiers.

    - ce code me parrait bien compliqué pour faire peu de chose, pe ut-on
    le simplifier (tout en gardant la gestion des erreurs) ?



On peut surtout mieux le structurer. Il faut aussi séparer le lecture
des traitements...


J'ai bien essayé de faire un point de sortie unique de la fonction mais
le code se retrouve inexorablement collé à droite de l'écran (et
j'utilise des indentations de quatre espaces).



Utilise 3 espaces. Si ça déborde encore, c'est qu'il n'y a pas assez
de découpage en fonctions...
Avatar
-ed-
On 15 mar, 12:09, (Marc Espie) wrote:

- Relis bien la doc de strncat, on peut arguer que cette fonction ne sert a
rien, en particulier, elle ne fait pas exactement ce que tu crois.

Elle copie effectivement buf vers line, sans depasser de buf_size, mais e lle
n'ajoute pas de 0 final si tu arrives au bout.



Euh, si ! Tu confonds avec strncpy(). Par contre avec strncat(), il
fait faire attention à ce que la chaine de destination soit
correctement initialisée.
Avatar
Xavier Roche
Benoit Izac a écrit :
char *basedir = "/sys/class/power_supply";



Histoire de pinailler, j'aurais écrit
const char *basedir = "/sys/class/power_supply";

(ou même
const char *const basedir = "/sys/class/power_supply";)

En effet, les chaînes sont en général constantes (*), il n'y a donc pas
de raison de ne pas le dire. Cela permet aussi de repérer les erreurs
éventuelles, comme d'écrire dans une chaine constante.

/*
* generate filename
*/



Cela pourrait être encapsulé de manière plus zen:

/* Verification des arguments (ext. gcc) */
#ifdef __GNUC__
#define FORMAT_ARG(fmt, arglist)
__attribute__ ((format (printf, fmt, arglist)))
#else
#define FORMAT_ARG(fmt, arglist)
#endif

static char* my_printf(char *format, ...) FORMAT_ARG(1, 2);

static char* my_printf(char *format, ...) {
va_list args;
int len;
char *str;
va_start(args, format);
len = vsnprintf(NULL, 0, format, args);
if (len < 0) {
return NULL;
}
str = malloc(len + 1);
if (str == NULL) {
return NULL;
}
if (vsnprintf(str, len + 1, format, args) > len) {
free(str);
return NULL;
}
va_end(args);
return str;
}

et après:

char *const s = my_printf("coucou %s", foo);
..
free(s);

[ Note très personnelle: plutôt que de renvoyer NULL dans cette
fonction, j'aurais tendence a faire assert(0). Un problème d'allocation
de quelques octets est très probablement signe que quelque chose ne va
pas bien du tout (mémoire pleine). Le programme crashera en général peu
après. De plus gérer la combinatoire de cas d'erreur aussi bas niveau
est pour des projets importants source de complexité. Et faire assert(0)
est de toute manière identique à ce que le système fera si un problème
d'allocation de la pile survient, en cas de pénurie de mémoire. ]

Petite série de questions au passage :
- comment faites vous pour être sûr que les allocations soient bien
libérées ?



Sans vouloir troller sur fclc, c'est amha _le_ point positif du C++: on
oublie les problèmes de libération, de gestion des compteurs de
référence, etc.

En C, il faut être beaucoup plus strict, et coder de manière propre pour
éviter les fuites.

Pour éviter les soucis, moi j'ai une tendence a n'avoir qu'un seul
"return" dans une fonction donnée, et libérer les pointeurs au même
niveau, qui auront été initialisés à NULL au début de la fonction. Comme
ça, aucun risque d'oubli.

- ce code me parrait bien compliqué pour faire peu de chose, peut-on
le simplifier (tout en gardant la gestion des erreurs) ?



La clé, a mon très humble avis, c'est d'utiliser des fonctions pour tout
ce qui est rébarbatif et/ou source d'erreurs (ou ce complexité du code).
La fonction my_printf() est un exemple typique de chose sympa a avoir
"sous la main" pour ne plus s'embêter avec des problèmes d'intendence.
Avatar
Xavier Roche
Marc Espie a écrit :
- Relis bien la doc de strncat, on peut arguer que cette fonction ne sert a
rien, en particulier, elle ne fait pas exactement ce que tu crois.
Elle copie effectivement buf vers line, sans depasser de buf_size, mais elle
n'ajoute pas de 0 final si tu arrives au bout.



(Lapsus avec strncpy -- qui est une fonction à bannir définitivement)

Par contre strcat() est aussi dangereuse que strcpy(), car elle peut
provoquer très facilement un débordement.

Je suis étonné de voir que les extensions "BSD" comme strlcpy() et
strlcat() ne sont toujours pas standardisées POSIX. Ce sont pourtant des
alternatives évidentes et propres.
Avatar
espie
In article <gpipee$nne$,
Xavier Roche wrote:
Je suis étonné de voir que les extensions "BSD" comme strlcpy() et
strlcat() ne sont toujours pas standardisées POSIX. Ce sont pourtant des
alternatives évidentes et propres.



Tiens, on va relancer la flamewar habituelle...

C'est lie a ce connard de Ulrich Drepper, mainteneur de la glibc,
uber-programmer a ses heures.

Son argument, c'est que tous les programmeurs sont super-bons, et donc que
strlcpy comme strlcat ne servent a rien, parce que de toutes facons, elles
ne font "que" remplacer les problemes de buffer overflow par des problemes
de troncature de chaines. Et la simplicite du code de verification de
debordement obtenu en utilisant strlcpy et strlcat n'est pas utile, puisque
notre super-bon programmeur ne va jamais se planter dans son code de gestion
memoire de chaines de caracteres.

Evidemment, ca a un peu tendance a ignorer la realite... a savoir qu'il y a
plein de programmeurs normaux dans la nature (voire de mauvais programmeurs,
mais ca je suis d'accord qu'il faudrait les reconvertir en autre chose).

Mais ca explique que la glibc ne possede pas ces fonctions-la... et donc
Linux. Ces jours-ci, ca parait delicat de rajouter des trucs dans POSIX ou
Single Unix sans que ca soit dispo sous Linux.

Notons que la glibc mene ici un combat d'arriere-garde, puisque meme le
noyau linux contient un strlcpy et un strlcat. Et qu'une proportion importante
de projets ont adopte ces fonctions, au point que tres souvent, le script
de configuration "active" une copie locale de celles-ci si on est sous Linux.
Avatar
Xavier Roche
Xavier Roche a écrit :
En effet, les chaînes sont en général constantes (*)



J'ai oublié le (*): de mémoire, le standard impose que les chaînes
constantes soient.. constantes. Donc en principe:
char *foo = "bar";

devrait provoquer un message d'avertissement. Ce n'est souvent pas le
cas, pour des raisons historiques (trop de programmes utilisent des
char* pour références des const char)

static char* my_printf(char *format, ...) {



On remarquera aussi que j'ai oublié "va_end(args);" avant les return
NULL. C'est mal. (D'un autre côté va_end() est NOOP sur la plupart des
implémentations, mais ce n'est pas une raison)
Avatar
Benoit Izac
Bonjour,

le 15/03/2009 à 12:37, Xavier Roche a écrit dans le message
<gpip9j$nne$ :

const char *basedir = "/sys/class/power_supply";



Si je comprend bien, cela veut dire : je ne modifie pas
"/sys/class/power_supply".

(ou même const char *const basedir = "/sys/class/power_supply";)



Et là : je ne modifie pas "/sys/class/power_supply" et je ne modifie pas
non plus l'adresse contenue dans basedir. C'est bien cela ?

/* Verification des arguments (ext. gcc) */
#ifdef __GNUC__
#define FORMAT_ARG(fmt, arglist)
__attribute__ ((format (printf, fmt, arglist)))
#else
#define FORMAT_ARG(fmt, arglist)
#endif

static char* my_printf(char *format, ...) FORMAT_ARG(1, 2);

static char* my_printf(char *format, ...) {



Merci, il faut que je regarde ça à tête reposée.

Petite série de questions au passage :
- comment faites vous pour être sûr que les allocations soient bien
libérées ?



Sans vouloir troller sur fclc, c'est amha _le_ point positif du C++:
on oublie les problèmes de libération, de gestion des compteurs de
référence, etc.



J'ai déjà regardé du coté de C++ mais il me semble que, bien que la
gestion de la mémoire soit facilitée, il y a tellement de choses à coté
qui sont compliqués que pour mon usage, ce n'est pas viable.
Généralement lorsque je veux faire une petit programme rapidement
fonctionnel, j'utilise Perl et ça me convient parfaitement (je précise
pour les trolleurs fous que ne suis pas analyste-programmeur et que je
ne travaille pas dans l'informatique). En fait je m'intéresse au C car
c'est la base de la plupart des programmes que j'utilise et je
souhaiterai être de capable de les comprendre/modifier/corriger. Ce
programme par exemple est un remplacement de acpid adapté à mes
besoins (autant faire quelque chose d'utile).

En C, il faut être beaucoup plus strict, et coder de manière propre
pour éviter les fuites.

Pour éviter les soucis, moi j'ai une tendence a n'avoir qu'un seul
"return" dans une fonction donnée, et libérer les pointeurs au même
niveau, qui auront été initialisés à NULL au début de la fonction.
Comme ça, aucun risque d'oubli.



J'en suis bien conscient, mais je me retrouve vite avec trop
d'indentation. D'un autre coté, si j'utilise plein de fonctions, je me
retrouve à ne plus savoir ce qu'elles font (sans doute un manque
d'habitude). Mais effectivement, il me semble intéressant de se
construire une bibliothèque avec des fonctions bien définies et
documentées.

--
Benoit Izac
1 2