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
Francois Lafont
Le 15/07/2014 09:24, Marc Espie a écrit :
Ou, si tu preferes, ne confond pas transtypage implicite et cast explicite.



Ok, je crois bien qu'en fait je ne soupçonnais même pas
que le C pouvait faire du transtypage implicite.

Tu as parfaitement le droit de convertir des valeurs d'un type a un autre
sans mettre de cast en C, et habituellement sans que le compilo moufte,
sauf cas particulier.

Ainsi,

void *p;

...

int *k = p;


ne va pas balancer de warning du tout. C'est totalement legal en C.

Tu reserves les cast aux cas ou ca chouine un peu.

Si ca passe sans le moindre avertissement du compilo, il ne faut surtout
pas mettre de cast, car les cast desactivent *completement* les
avertissements. [...]



Ok, merci beaucoup pour toutes ces explications.

Du coup, je copie ici une version légèrement modifiée
de mon code, où j'ai supprimé tous les cast inutiles.
Il en reste plus qu'un seul dont je ne peux pas me
débarrasser au niveau de la ligne :

curl_easy_setopt(curl, CURLOPT_WRITEDATA, (void *) wr_buf);

mais je ne peux rien y faire car c'est la fonction
curl_easy_setopt de la lib curl qui est comme ça.

Merci encore.

----------------------------------------------------
/* With Debian Wheezy:
*
* sudo apt-get install libcurl4-openssl-dev
* gcc -stdÉ9 -Wextra -Wall -O2 -o sp_check sp_check.c -lcurl
*
*/

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

#define MAX_BUF_IN_BYTES 65536
#define MAX_POST_IN_BYTES 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_IN_BYTES] = { 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_IN_BYTES) {
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_IN_BYTES + 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, "0n", 2)) {
return_value = OK;
}
else if (!strncmp(wr_buf, "1n", 2)) {
return_value = WARNING;
}
else if (!strncmp(wr_buf, "2n", 2)) {
return_value = CRITICAL;
}
else if (!strncmp(wr_buf, "3n", 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) {

// We want to keep the value of wr_index for the next call
// this function. So, wr_index is a static variable.
static int wr_index = 0;

// The size (in bytes) of the received buffer.
int segsize = size * nmemb;

// In this function, userp refers to a simple array of bytes.
char *wr_buf = userp;

// 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_IN_BYTES) {
if (MAX_BUF_IN_BYTES - wr_index > 0) {
memcpy(&wr_buf[wr_index], buffer, MAX_BUF_IN_BYTES - wr_index);
}
wr_index = MAX_BUF_IN_BYTES + 1; // wr_buf will be not written anymore.
return segsize;
}

// Copy the data from the curl buffer into our buffer.
memcpy(&wr_buf[wr_index], buffer, 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 == '') {
return 0;
}

size_t 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;
}
----------------------------------------------------


--
François Lafont
Avatar
Xavier Roche
Quelques remarques rapides (avec très possiblement des erreurs ou des
bourdes hein)

Le 15/07/2014 14:02, Francois Lafont a écrit :
int isPositiveInteger(const char s[]);



Préférence toute personnelle: je préfère const char *s (mais bon)

int timeout = atoi(argv[1]);



Là aussi, j'ai un réflexe d'écrire:
const int timeout = atoi(argv[1]);

Ça ne coûte rien (àa la longue, c'est câblé dans les doigts), et cela
peut occasionnellement éviter les ennuis (la variable n'a pas besoin
d'être modifiée, donc on l'indique au compilateur) - du genre "je me
suis emmêlé les pinceaux entre deux variables"

char *url = argv[2];



De même (le pointeur n'a pas de raison d'être changé)
char *const url = argv[2];

ou mieux (mais je ne sais pas si CURL prend bien un const char*)
const char *const url = argv[2];

char post[MAX_POST_IN_BYTES] = { 0 };



Équivaut à un memset(post, 0, sizeof(post)), donc un poil coûteux.
Est-il indispensable ?

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);



Euh, c'est du C99 char temp[temp_size] ? :)

Sinon asprintf() (-D_GNU_SOURCE) est une extension GNU intéressante sous
Linux. (note: asnprintf() peut être facilement réécrit à coup de malloc
et de snprintf)

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



memset(wr_buf, 0, sizeof(wr_buf)) :p

// Copy the data from the curl buffer into our buffer.



Mode parano, j'aurais collé un petit assert avant du genre:
assert(wr_index <= MAX_BUF_IN_BYTES && segsize < MAX_BUF_IN_BYTES -
wr_index);
memcpy(&wr_buf[wr_index], buffer, segsize);



On sait jamais, ça permet de dormir tranquille.

int isPositiveInteger(const char s[]) {
...
size_t i;
for (i = 0; i < strlen(s); i++) {



Ciel. Il vaut mieux:

for (i = 0; s[i] != '' ; i++) {

Histoire d'éviter de la complexité quadratique si le compilateur a pas
rattrapé.

// ASCII value of 0 -> 48, of 1 -> 49, ..., of 9 -> 57.
if (s[i] < 48 || s[i] > 57) {



Ou, un peu plus clair:
if (s[i] < '0' || s[i] > '9') {
Avatar
Olivier Miakinen
Le 15/07/2014 22:24, Xavier Roche a écrit :

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



Ciel. Il vaut mieux:

for (i = 0; s[i] != '' ; i++) {



Oui.

Ou :
for (i = 0; s[i] != 0 ; i++) {

Ou encore :
for (i = 0; s[i] ; i++) {

Les trois écritures font exactement la même chose (contrairement
au strlen si le compilateur n'optimise pas), c'est juste une
question de goût.

// ASCII value of 0 -> 48, of 1 -> 49, ..., of 9 -> 57.
if (s[i] < 48 || s[i] > 57) {



Ou, un peu plus clair:
if (s[i] < '0' || s[i] > '9') {



Oui.
Avatar
Lucas Levrel
Le 15 juillet 2014, Xavier Roche a écrit :

int isPositiveInteger(const char s[]);



Préférence toute personnelle: je préfère const char *s (mais bon)



s ne serait-il pas const dans le premier cas et pas dans le second ?

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



Ciel. Il vaut mieux:

for (i = 0; s[i] != '' ; i++) {

Histoire d'éviter de la complexité quadratique si le compilateur a pas
rattrapé.



Tu veux dire que strlen(s) pourrait être évalué à chaque tour de
boucle ? Même si s est const char *const ? (Bien sûr c'est théoriquement
possible, mais un compilateur normal ne va-t-il pas optimiser à coup sûr
compte tenu des deux const ?)


--
LL
Ἕν οἶδα ὅτι οὐδὲν οἶδα (Σωκράτης)
Avatar
espie
In article ,
Lucas Levrel wrote:
Le 15 juillet 2014, Xavier Roche a écrit :

int isPositiveInteger(const char s[]);



Préférence toute personnelle: je préfère const char *s (mais bon)



s ne serait-il pas const dans le premier cas et pas dans le second ?

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



Ciel. Il vaut mieux:

for (i = 0; s[i] != '' ; i++) {

Histoire d'éviter de la complexité quadratique si le compilateur a pas
rattrapé.



Tu veux dire que strlen(s) pourrait être évalué à chaque tour de
boucle ? Même si s est const char *const ? (Bien sûr c'est théoriquement
possible, mais un compilateur normal ne va-t-il pas optimiser à coup sûr
compte tenu des deux const ?)



Non, le fait que ca soit const char * n'a rien a voir avec tout ca.

strlen fait partie de la bibliotheque standard -> un compilo "moderne"
est susceptible de connaitre parfaitement sa semantique et de l'inliner,
et donc de le faire completement disparaitre.

Mais ca n'a rien de garantie. strlen(s) n'est pas specialement une constante.
Si on regarde la definition du proto de la fonction, un compilo qui ne
fait rien de magique pour strlen n'en saura pas plus.

(note que c'est un truc que C++2011 corrige avec constexpr, qui leur a
permis d'etendre fortement les regles de constantes pour la compilation,
mais qui ne s'applique absolument pas au C)
Avatar
Xavier Roche
On 07/16/2014 02:06 PM, Marc Espie wrote:
(note que c'est un truc que C++2011 corrige avec constexpr, qui leur a
permis d'etendre fortement les regles de constantes pour la compilation,
mais qui ne s'applique absolument pas au C)



Cependant il y a des cas ou cela ne suffira pas ; par exemple si un
appel de fonction est effectué dans la boucle.

Exemple hypothétique:

const char *const s = ma_variable_globale;
for (i = 0; i < strlen(s); i++) {
...
ma_fonction();
}

...

void ma_fonction() {
ma_variable_globale[0] = ''; /* apu */
}

Cet exemple est parfaitement valide, et justifie l'emploi du strlen() à
chaque test de boucle. Le fait que les données changent ne violent en
rien le standard (pas d'aliasing interdit typiquement)

Bon, l'exemple est capillotracté, mais cela montre les limites qu'un
compilateur peut atteindre, même quand il essaye d'être malin.
Avatar
Xavier Roche
On 07/16/2014 01:57 PM, Lucas Levrel wrote:
Préférence toute personnelle: je préfère const char *s (mais bon)


s ne serait-il pas const dans le premier cas et pas dans le second ?



Exactement. Dans le premier cas, le pointeur peut changer, même si les
données en dessous sont toujours constantes. Dans le second cas, le
pointeur ne PEUT PAS changer, ce qui donne une sécurité supplémentaire.

Tu veux dire que strlen(s) pourrait être évalué à chaque tour de boucle
? Même si s est const char *const ?



Le fait que cette variable soit constante ne veut pas dire que les
données en dessous le soient véritablement. Voir mon exemple juste
avant, où s pointerait sur un buffer global, modifié par un appel de
fonction dans la boucle.
Avatar
Benoit Izac
Bonjour,

le 15/07/2014 à 22:24, Xavier Roche a écrit dans le message
<lq42lq$bpt$ :

// ASCII value of 0 -> 48, of 1 -> 49, ..., of 9 -> 57.
if (s[i] < 48 || s[i] > 57) {



Ou, un peu plus clair:
if (s[i] < '0' || s[i] > '9') {




Ou encore (plus portable ?) :
if (isdigit(s[i])) {

--
Benoit Izac
Avatar
Francois Lafont
Bonsoir,

Le 15/07/2014 22:24, Xavier Roche a écrit :
Quelques remarques rapides (avec très possiblement des erreurs ou des
bourdes hein)



Pas de souci. ;)

int isPositiveInteger(const char s[]);



Préférence toute personnelle: je préfère const char *s (mais bon)



Ah, en fait je m'étais posé la question et je me suis
dit qu'en pratique l'argument effectif de cette fonction
serait un tableau de char (qui sera certes converti en
un pointeur de type char*) et que donc c'était plus
clair de mettre « const char s[] ».

Mais bon, si j'ai bien compris c'est totalement
équivalent.

int timeout = atoi(argv[1]);



Là aussi, j'ai un réflexe d'écrire:
const int timeout = atoi(argv[1]);

Ça ne coûte rien (àa la longue, c'est câblé dans les doigts), et cela
peut occasionnellement éviter les ennuis (la variable n'a pas besoin
d'être modifiée, donc on l'indique au compilateur) - du genre "je me
suis emmêlé les pinceaux entre deux variables"

char *url = argv[2];



De même (le pointeur n'a pas de raison d'être changé)
char *const url = argv[2];

ou mieux (mais je ne sais pas si CURL prend bien un const char*)
const char *const url = argv[2];



Ok. Ça passe à la compilation (curl ne semble pas
poser problème). Je suis d'accord pour ces mesures
de prévention qui ne coûtent pas chers effectivement.

char post[MAX_POST_IN_BYTES] = { 0 };



Équivaut à un memset(post, 0, sizeof(post)), donc un poil coûteux.
Est-il indispensable ?



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.

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);



Euh, c'est du C99 char temp[temp_size] ? :)



Oui et je dois dire que c'est assez commode de pouvoir
déclarer des tableaux dont la taille est une variable.
Je me suis dit qu'en 2014 je pouvais faire du c99. :)
Du moment que ça passe sur ma Wheezy, ça me va.

Sinon asprintf() (-D_GNU_SOURCE) est une extension GNU intéressante sous
Linux. (note: asnprintf() peut être facilement réécrit à coup de malloc
et de snprintf)

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



memset(wr_buf, 0, sizeof(wr_buf)) :p



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

// Copy the data from the curl buffer into our buffer.



Mode parano, j'aurais collé un petit assert avant du genre:
assert(wr_index <= MAX_BUF_IN_BYTES && segsize < MAX_BUF_IN_BYTES -
wr_index);
memcpy(&wr_buf[wr_index], buffer, segsize);



On sait jamais, ça permet de dormir tranquille



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

int isPositiveInteger(const char s[]) {
...
size_t i;
for (i = 0; i < strlen(s); i++) {



Ciel. Il vaut mieux:

for (i = 0; s[i] != '' ; i++) {

Histoire d'éviter de la complexité quadratique si le compilateur a pas
rattrapé.



Et je fais ça plutôt :

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

Ça irait ?

// ASCII value of 0 -> 48, of 1 -> 49, ..., of 9 -> 57.
if (s[i] < 48 || s[i] > 57) {



Ou, un peu plus clair:
if (s[i] < '0' || s[i] > '9') {



Ah oui, c'est bien ça.

Merci bien pour toutes ces remarques.

--
François Lafont
Avatar
espie
In article <lq6oh3$u40$,
Olivier Miakinen <om+ wrote:
Le 16/07/2014 22:18, Benoit Izac a écrit :

// ASCII value of 0 -> 48, of 1 -> 49, ..., of 9 -> 57.
if (s[i] < 48 || s[i] > 57) {



Ou, un peu plus clair:
if (s[i] < '0' || s[i] > '9') {



Ou encore (plus portable ?) :
if (isdigit(s[i])) {



En l'occurrence, ce serait plutôt :
if (! isdigit(s[i])) {

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...
1 2 3 4 5