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

Petit programme en C, avez-vous des remarques etc. ?

Aucune réponse
Avatar
Francois Lafont
Bonjour à tous,

Autant le dire tout de suite, je suis vraiment totalement
inexpérimenté en langage C (pour ne pas dire plus). Ceci,
étant dit, j'ai codé un petit programme et j'aimerais bien
avoir, si c'est possible, vos suggestions et critiques. Je
mets le programme en fin de message.

C'est un programme qui se lance en ligne de commandes de cette
manière :

./mon-programme 10 localhost/cgi/machin.pl xxx yyy zzz

Le programme va alors lancer une requête http à l'adresse
http://localhost/cgi/machin.pl en utilisant la lib cURL
(ici, il s'agit d'un script Perl en localhost mais ça pourrait
être autre chose). La valeur 10 correspond à un timeout en
secondes (au delà le programme abandonne la requête), et le
reste des arguments xxx, yyy, zzz (il doit y en avoir au moins
1, mais ce n'est pas forcément 3 arguments, le nombre d'arguments
dépend de la page qui est appelée) est passé en variable POST
à la page http qui est appelée. Les variables POST seront
définies sous cette forme :

token1=xxx
token2=yyy
token3=zzz

Ensuite, les pages qui seront appelées avec ce programme
devront fournir une sortie de la forme :

- une première avec juste le nombre 0, 1, 2 ou 3;
- et des lignes suivantes avec ce qu'on veut, peu importe.

Par exemple :

2
CRITICAL blabla blabla.
Blibli blibli.

est une sortie correcte pour la page web. Ensuite, le
programme, qui aura mis cette sortie dans un buffer,
affichera uniquement les lignes autres que la ligne 1
et devra retourner comme exit code la valeur indiquée
dans la ligne 1 de la sortie de la page web. Si je
reprends l'exemple précédent, le programme devra donc
dans ce cas là afficher en sortie :

CRITICAL blabla blabla.
Blibli blibli.

tout en retournant la valeur 2 comme exit code.
Enfin, si la sortie de la page web appelée est trop
longue, alors le programme devra tronquer cette
sortie suivant une taille limite qui est indiquée
en tant que macro dans le programme.

Voilà pour ce que le programme est censé faire.

Pour l'écrire, je suis parti d'exemples que l'on
trouve ici ou là sur le web à propos de la lib cURL.
Par exemple, un point en particulier sur lequel
j'aimerais bien des avis (même si je suis vraiment
preneur de toute remarque à propos du code), c'est
l'usage du "cast". J'ai souvent entendu dire que
c'était à éviter et qu'on pouvait en général s'en
passer. Or dans le code, le cast est pas mal utilisé.
Peut-être que je m'y suis mal pris... Je n'ai pas vu
comment faire autrement par exemple au niveau d'une
fonction qui attend en argument des pointeurs de type
void*. En effet, dans le code on indique que l'on
souhaite écrire la sortie de la requête cURL dans un
buffer en indiquant le nom d'une fonction write_data
qui doit avoir cette signature :

size_t write_data(void *buffer, size_t size, size_t nmemb, void *userp);

où :
- la variable buffer un pointeur vers un buffer qui
contient un bout de la sortie de la requête cURL,
- size*nmemb correspond à la taille de ce buffer,
- userp est un pointeur vers le buffer dans lequel
on souhaite écrire la sortie de cURL (c'est ce
buffer là que l'on récupère dans la fonction main).

Dans la fonction main, userp correspond à la variable
wr_buf qui est un tableau de "char" mais il doit être
casté en void* pour être passé à la fonction write_data,
sachant que, dans le corps de la fonction write_data, je
m'empresse de faire le cast « inverse » (ie je caste
userp en un pointeur de type char*). Voilà par exemple
un point sur lequel je m'interroge. Mais encore une
fois, toutes vos remarques (sur la forme comme sur le
fond) m'intéressent.

Au départ, je pensais que ce programme allait réellement
me servir mais finalement il est probable que je ne l'utilise
pas. Ceci étant, en écrivant ce programme, je me suis pris
au jeu en quelques sortes et j'ai tenté de l'achever tant
bien que mal.

Merci d'avance pour toutes vos remarques et critiques.
François Lafont

-------------------------------------------------------
/* With Debian Wheezy:
*
* sudo apt-get install libcurl4-openssl-dev
* gcc -lcurl -std=c99 -o curl-launcher.exe curl-launcher.c
*
*/

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <ctype.h>
#include <curl/curl.h>

#define MAX_BUF 65536
#define MAX_POST_LENGTH 4096
#define OK 0
#define WARNING 1
#define CRITICAL 2
#define UNKNOWN 3

size_t write_data(void *buffer, size_t size, size_t nmemb, void *userp);
int isPositiveInteger(const char s[]);

int main(int argc, char *argv[]) {

if (argc < 4) {
printf("Sorry, bad syntax. You must apply at least 3 arguments.\n");
printf("%s <timeout> <url> <args>...\n", argv[0]);
return UNKNOWN;
}

if (!isPositiveInteger(argv[1])) {
printf("Sorry, bad syntax. The first argument must be a positive");
printf(" integer (it's a timeout in seconds).\n");
return UNKNOWN;
}

int timeout = atoi(argv[1]);
char *url = argv[2];

// First step, init curl.
CURL *curl;
curl = curl_easy_init();

if (!curl) {
printf("Sorry, couldn't init curl.\n");
return UNKNOWN;
}

// Construction of the post variable, a string with this form:
// token1=<urlencoded data1>&token2=<urlencoded data2>&...
char post[MAX_POST_LENGTH] = { 0 };
int token_num = 1;
char *urlencoded_str = NULL;
int i = 0;

for (i = 3; i < argc; i++) {

if (token_num > 999) {
printf
("Sorry, the limit number (999) of POST variables is exceeded.\n");
curl_easy_cleanup(curl);
return UNKNOWN;
}

//printf("C: token%d: [%s]\n", token_num, argv[i]);

urlencoded_str = curl_easy_escape(curl, argv[i], 0);

// 10 is the max length of the string "token<num>=&".
// The maximum is reached with "token999=&".
int temp_size = 10 + strlen(urlencoded_str) + 1;
char temp[temp_size];
//memset(temp, 0, temp_size*sizeof(char));
sprintf(temp, "token%d=%s&", token_num, urlencoded_str);

if (strlen(post) + strlen(temp) + 1 < MAX_POST_LENGTH) {
strcat(post, temp);
}
else {
printf("Sorry, the max POST size is exceeded.\n");
curl_easy_cleanup(curl);
return UNKNOWN;
}

curl_free(urlencoded_str);
token_num++;

}

// Remove the last character "&".
post[strlen(post) - 1] = 0;
//printf("C: POST [%s]\n", post);

char wr_buf[MAX_BUF + 1] = { 0 };

curl_easy_setopt(curl, CURLOPT_URL, url);
curl_easy_setopt(curl, CURLOPT_TIMEOUT, timeout);
curl_easy_setopt(curl, CURLOPT_POSTFIELDS, post);

// Tell curl that we'll receive data to the function write_data
// which will write the data in wr_buf.
curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, write_data);
curl_easy_setopt(curl, CURLOPT_WRITEDATA, (void *) wr_buf);

// Allow curl to perform the action.
CURLcode ret;
ret = curl_easy_perform(curl);

if (ret) {
curl_easy_cleanup(curl);
printf("Sorry, exit value of curl is %d.", ret);

switch (ret) {

case CURLE_COULDNT_RESOLVE_HOST:
printf(" Could not resolve the host address.\n");
break;

case CURLE_OPERATION_TIMEDOUT:
printf(" Operation timeout.\n");
break;

default:
printf("\n");
break;

}

return UNKNOWN;
}

curl_easy_cleanup(curl);

/*
printf("----------------------------------\n");
printf("%s", wr_buf);
printf("----------------------------------\n");
*/

int return_value;
if (!strncmp(wr_buf, "0\n", 2)) {
return_value = OK;
}
else if (!strncmp(wr_buf, "1\n", 2)) {
return_value = WARNING;
}
else if (!strncmp(wr_buf, "2\n", 2)) {
return_value = CRITICAL;
}
else if (!strncmp(wr_buf, "3\n", 2)) {
return_value = UNKNOWN;
}
else {
printf("Unexpected output of the plugin, return value not");
printf(" displayed or not in {0, 1, 2, 3}.\n");
return UNKNOWN;
}

printf("%s", wr_buf + 2);
return return_value;

}

// Write data callback function (called within the context
// of curl_easy_perform).
size_t write_data(void *buffer, size_t size, size_t nmemb, void *userp) {

// It is assumed that the type of userp is char*.
char *wr_buf = (char *) userp;

static int wr_index = 0;
int segsize = size * nmemb;

// Check to see if this data exceeds the size of our buffer. If so,
// it's possible to return O to indicate a problem to curl.
// But here, we just stop the function without error (ie, we return
// segsize) and our buffer will be troncated.
if (wr_index + segsize > MAX_BUF) {
if (MAX_BUF - wr_index > 0) {
memcpy((void *) &wr_buf[wr_index], buffer,
(size_t) (MAX_BUF - wr_index));
}
wr_index = MAX_BUF + 1; // wr_buf will be not written anymore.
return segsize;
}

// Copy the data from the curl buffer into our buffer.
memcpy((void *) &wr_buf[wr_index], buffer, (size_t) segsize);

// Update the write index.
wr_index += segsize;

// Return the number of bytes received, indicating to curl that
// all is okay.
return segsize;
}

int isPositiveInteger(const char s[]) {

if (s == NULL || *s == '\0') {
return 0;
}

int i;
for (i = 0; i < strlen(s); i++) {
// ASCII value of 0 -> 48, of 1 -> 49, ..., of 9 -> 57.
if (s[i] < 48 || s[i] > 57) {
return 0;
}
}

return 1;
}
-------------------------------------------------------

10 réponses

1 2 3 4 5
Avatar
Olivier Miakinen
Le 17/07/2014 01:00, Marc Espie a écrit :

Je ne pense pas que ce soit ni plus ni moins portable. C'est peut-être,
cela dit, un quart de poil de chouia de microseconde plus rapide ?



Ah si, c'est plus portable. Il y a des codages tres bizarres dans la
nature. Typiquement, en EBCDIC, les chiffres sont tout en haut. Suis
pas sur que ca donne des choses tres coherentes avec des char signes
(ce qui je te l'accorde serait un peu loufoque dans ce codage).

Note que par exemple, pour les lettres, les codes ne sont pas contigus...



Néanmoins, je persiste et je signe.

Il s'agit d'un programme qui utilise curl pour faire des requêtes
http. Si pour un tel programme on devait utiliser un charset non
compatible avec ASCII, pour le coup il ne serait pas seulement
non-portable, il serait complètement cassé !

Et donc, je le redis, utiliser isdigit() ne sera ni plus ni moins
portable que de comparer les octets avec '0' et '9'. À la limite,
si on veut vraiment le rendre portable sur un système EBCDIC, il
vaudrait mieux comparer avec 48 et 57 plutôt qu'avec '0' et '9' !

Cordialement,
--
Olivier Miakinen
Avatar
Xavier Roche
On 07/17/2014 12:04 AM, Francois Lafont wrote:
On peut peut-être s'en passer mais il faut alors
que je modifie le code. Avec cette initialisation,
je suis sûr que "post" sera à chaque moment une
chaîne de caractère valide avec forcément un
à un moment donné et je m'en sers dans le code.



Mais un:
post[0] = '';
suffit à priori, non ?

A partir du moment où ce tableau est une chaîne valide terminée par ,
toute opération de type strcat(), etc. garde cet invariant.

Même chose ici, en l'état je ne crois pas que je puisse
me passer de cette initialisation.



Il suffirait (à priori en lisant le code rapidement), dans write_data(),
après le memcpy(), de coller un .

A ce sujet, write_data() prend un "userp" opaque, qui permet de ne pas
avoir d'horribles variables statiques. Plutôt que de passer "juste" le
buffer, on peut passer une structure avec plus d'infos:

typedef struct write_data_struct {
char *buf; /* buffer */
size_t index; /* offset to data */
size_t capa; /* buffer capacity */
} write_data_struct;

Et dans le code:

...
write_data_struct wr;
wr.buf = wr_buf;
wr.index = 0;
wr.capa = MAX_BUF_IN_BYTES;

...
curl_easy_setopt(curl, CURLOPT_WRITEDATA, (void *) &wr);

Et dans la fonction:

size_t write_data(void *buffer, size_t size, size_t nmemb, void *userp) {
write_data_struct *const wr = (write_data_struct*) userp;
const size_t segsize = size * nmemb;

... et utiliser wr->index en place de 'wr_index', wr->capa en place de
'MAX_BUF_IN_BYTES', et wr->buf en place de ''wr_buf

Mais le assert n'est-il pas consommateur de ressources ?
Car je suppose qu'il sera testé à chaque appel de la
fonction, non ?



Non, cela ne coûte rien. Éventuellement on se pose la question dans une
boucle interne de copie de bitmap, par exemple, mais dans une fonction
qui est appelée dans le cadre de transferts réseau, un test de taille,
c'est totalement transparent.

Préférence personnelle également: je compile les assert() y compris en
"release". Sinon ça sert à rien, un assert.

#undef assert
#define assert(exp) (void) ( (exp) || (my_abort_function(#exp, __FILE__,
__LINE__), 0) )

[ ou, avec GCC, pour indiquer que la condition est en général vraie,
mais là je chipote vraiment un peu:

#ifdef __GNUC__
#define assert(exp) (void) ( (__builtin_expect((exp), 1)) ||
(my_abort_function(#exp, __FILE__, __LINE__), 0) )
#endif
]

size_t i;
size_t N = strlen(s);
for (i = 0; i < N; i++) {...

Ça irait ?



Oui, mais j'aurais écrit 'const size_t N = ...' :p

Et cela reste plus lent de toute manière: on parcours deux fois la
chaîne, au lieu d'une. Et strlen() n'est fondamentalement rien d'autre que:

size_t strlen(const char *const s) {
size_t i;
for (i = 0; s[i] != '' ; i++) ;
return i;
}
Avatar
espie
In article <lq7o3r$1h58$,
Olivier Miakinen <om+ wrote:
Néanmoins, je persiste et je signe.



Ouais, t'es juste un peu buté. :-)

Il s'agit d'un programme qui utilise curl pour faire des requêtes
http. Si pour un tel programme on devait utiliser un charset non
compatible avec ASCII, pour le coup il ne serait pas seulement
non-portable, il serait complètement cassé !

Et donc, je le redis, utiliser isdigit() ne sera ni plus ni moins
portable que de comparer les octets avec '0' et '9'. À la limite,
si on veut vraiment le rendre portable sur un système EBCDIC, il
vaudrait mieux comparer avec 48 et 57 plutôt qu'avec '0' et '9' !



Bon, apres, une autre optique, c'est d'apprendre les idiomes les plus
simples et les plus reguliers possibles. Et dans cette optique,
l'utilisation de isdigit() est clairement un progres sur la comparaison
explicite a des codes numeriques...
Avatar
Lucas Levrel
Le 17 juillet 2014, Xavier Roche a écrit :

size_t write_data(void *buffer, size_t size, size_t nmemb, void *userp) {
write_data_struct *const wr = (write_data_struct*) userp;



Le cast de void* c'est Mal (© Marc Espie).

--
LL
Ἕν οἶδα ὅτι οὐδὲν οἶδα (Σωκράτης)
Avatar
Xavier Roche
On 07/17/2014 09:28 AM, Lucas Levrel wrote:
Le cast de void* c'est Mal (© Marc Espie).



Non, là c'est zen, car on caste un void* qui est du même type, donc
c'est parfaitement autorisé et défini.

Et c'est la méthode classique pour tout ce qui prend un callback
(fonction avec pointeur opaque)

Bon, évidemment, il faut être sûr du type :)
Avatar
espie
In article <lq7vtj$69e$,
Xavier Roche wrote:
On 07/17/2014 09:28 AM, Lucas Levrel wrote:
Le cast de void* c'est Mal (© Marc Espie).



Non, là c'est zen, car on caste un void* qui est du même type, donc
c'est parfaitement autorisé et défini.

Et c'est la méthode classique pour tout ce qui prend un callback
(fonction avec pointeur opaque)

Bon, évidemment, il faut être sûr du type :)



Ben precisement, t'as pas besoin du cast. Un transtypage implicite marche.
Avatar
Xavier Roche
On 07/17/2014 10:22 AM, Marc Espie wrote:
Ben precisement, t'as pas besoin du cast. Un transtypage implicite marche.



Oui, pas faux (en C en tout les cas)
Avatar
espie
In article <lq82m7$69e$,
Xavier Roche wrote:
On 07/17/2014 10:22 AM, Marc Espie wrote:
Ben precisement, t'as pas besoin du cast. Un transtypage implicite marche.



Oui, pas faux (en C en tout les cas)




Ah mais totalement. De toutes facons le code ecrit pour "marcher en C comme
en C++" est une mauvaise idee: tu rajoutes des cast pourris qui sont une
source potentielle d'erreur en C, et tu te retrouves au final avec du
code moins sur.

Si on veut passer en C++, autant imposer d'office un modele de typage propre.
Donc si on a du callback a base de void*, ben ca se wrappe tres bien dans
du template (un peu comme la factorisation pour implementer
vector<T*> sur vector<void *>) et on se debarrasse VRAIMENT des problemes
de typage sur le code client.

Et pour les cast encore necessaires, const_cast<>, static_cast<> et
dynamic_cast<> sont nos amis.
Avatar
Olivier Miakinen
Le 17/07/2014 09:11, Marc Espie m'a répondu :

Néanmoins, je persiste et je signe.



Ouais, t'es juste un peu buté. :-)



Oui. :-)

Mais c'est un entêtement réfléchi.

Tu m'as cité un cas où isdigit() pourrait être préférable à
((n >= '0') && (n <= '9')) : l'utilisation, sur une machine ne
communiquant pas avec l'extérieur, d'un charset plus stupide
que ASCII et même que EBCDIC, où comme pour les lettres dans
EBCDIC les chiffres ne seraient pas contigus.

Puisqu'on fait dans l'exotique, voici un cas où comparer la
valeur avec '0' et '9' pourrait être préférable à isdigit() :
ce serait un système dans lequel isdigit() retourne vrai
lorsque la propriété Unicode 'Nombre, chiffre décimal' est
vraie (p{Nd}) :
<http://www.fileformat.info/info/unicode/category/Nd/list.htm>.

Bon, apres, une autre optique, c'est d'apprendre les idiomes les plus
simples et les plus reguliers possibles. Et dans cette optique,
l'utilisation de isdigit() est clairement un progres sur la comparaison
explicite a des codes numeriques...



Là, d'accord. Cela ne dispense pas de s'assurer de ce que fait
isdigit() -- et de vérifier dans quel charset on travaille -- mais
oui, c'est une bonne raison pour utiliser isdigit().

Cordialement,
--
Olivier Miakinen
Avatar
Olivier Miakinen
Le 17/07/2014 10:22, Marc Espie cité de l'UTF-8 :

Bon, évidemment, il faut être sûr du type :)





Ah, je comprends ton attachement à l'EBCDIC. :-)
1 2 3 4 5