Potential confusion in http_proxy.c and a recommendation

From: Hasabnis, Niranjan via curl-library <curl-library_at_cool.haxx.se>
Date: Mon, 9 Nov 2020 19:04:13 +0000

Hello CURL developers,

My name is Niranjan and I work at Intel. We were recently analyzing various open-source GitHub repositories for idiosyncratic programming patterns. We came across “if (s->keepon > TRUE)” pattern at line 360 in lib/http_proxy.c, and we believe that this pattern is a bit confusing, and can lead to a potential issue in the future. We wanted to have your opinion about it.

Here are more details about this pattern. keepon is defined as int in struct http_connect_state in lib/urldata.h,



/* struct for HTTP CONNECT state data */

struct http_connect_state {

  …

  int keepon;

  …
};

while TRUE is defined as true in curl_setup_once.h.



#ifndef TRUE

#define TRUE true

#endif

#ifndef FALSE

#define FALSE false
#endif

It seems that “if (s->keepon > TRUE)” code intends to capture the semantics of “s->keepon > 1”, as true is defined as integer 1 in C language. In fact, lib/http_proxy.c contains “s->keepon = 2;”

We believe that the aforementioned pattern, however, causes confusion because any non-zero value can be considered as true in C language. In other words, even a negative value of keepon can satisfy the condition if we consider this notion. We believe that using “if (s->keepon > 1)” would eliminate this confusion and capture the intended semantics precisely.
Do you have any comments on this recommendation and the potential confusion we mentioned?

Thanks,
Niranjan.



-------------------------------------------------------------------
Unsubscribe: https://cool.haxx.se/list/listinfo/curl-library
Etiquette: https://curl.haxx.se/mail/etiquette.html
Received on 2020-11-09