Fix function prototype issue

Amrnasr / lz4

Automatically exported from code.google.com/p/lz4

0 stars 0 forks source link

Fix function prototype issue #116

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago

Declaration of functions without argument in lz4.h and lz4hc.h is not a 
prototype. People can accidentally call this function with extra argument which 
is confusing.

Original issue reported on code.google.com by sunxiaoguang@gmail.com on 22 Feb 2014 at 12:38

Attachments:

prototype.patch

GoogleCodeExporter commented 8 years ago

OK. Not sure if it is a big issue (I personally like the simple () )
but the fix is simple enough, 
so as long as no compiler complain about, that's a fine addition.

Original comment by yann.col...@gmail.com on 27 Feb 2014 at 7:06

Changed state: Accepted

GoogleCodeExporter commented 8 years ago

The attached file
is a wip r114 package
which implements your recommended changes
into *.h files.

Original comment by yann.col...@gmail.com on 27 Feb 2014 at 10:33

Attachments:

lz4-r114.tar.gz

GoogleCodeExporter commented 8 years ago

Thanks. This normally is not an issue when compiling C++ code, but in C, 
function prototype without argument means a function that can take arbitrary 
number of arguments. Which could be a problem, and if compile with -Wall you 
will see this warning. Since we use -Werror as well, so compiler stops at here.

Original comment by sunxiaoguang@gmail.com on 28 Feb 2014 at 1:31

GoogleCodeExporter commented 8 years ago

You're welcomed.

As a minor comment : note that the current Makefile of LZ4 uses -Wall argument, 
however it has never complained (warned) about such issue, on none of my 
systems.

Maybe this particular warning requires more than -Wall to be triggered ? For 
example, I already had to add -Wundef to get additional warnings that -Wall was 
not providing.

Original comment by yann.col...@gmail.com on 28 Feb 2014 at 1:36

GoogleCodeExporter commented 8 years ago

Sorry, It was my mistake. The flags caused compiler to scream is 
-Wstrict-prototypes. We build enterprise software and had to support many many 
operating systems so we tend to use very strict compiler settings to catch 
possible issues early. Thanks again to point out this.

Original comment by sunxiaoguang@gmail.com on 28 Feb 2014 at 1:47

GoogleCodeExporter commented 8 years ago

Fixed into r114

Original comment by yann.col...@gmail.com on 12 Mar 2014 at 2:56

Changed state: Fixed