stcorp / coda

The Common Data Access toolset
http://stcorp.github.io/coda/doc/html/index.html
BSD 3-Clause "New" or "Revised" License
39 stars 17 forks source link

Use C99 to reduce where variables are define and make them const when possible #48

Closed schwehr closed 4 years ago

schwehr commented 4 years ago

There are a lot of places where is would make static analysis easier if the code used C99 syntax. Doing this is typically pretty easy. While it doesn't seem like much, it makes debugging easier and makes compiler and static analyzers' output clearer.

I can do some of these as pull requests if that's okay with the project.

e.g.

uint32_t section_size;
// Lots of code
section_size = (((uint32_t)buffer[0] * 256 + buffer[1]) * 256 + buffer[2]) * 256 + buffer[3];

Could become:

// Lots of code
const uint32_t section_size =
    (((uint32_t)buffer[0] * 256 + buffer[1]) * 256 + buffer[2]) * 256 + buffer[3];

And

int i;
// Lots of code
for (i = 0; i < num_grib_types; i++)
{
    grib_type[i] = NULL;
}

Could be:

// Lots of code

for (int i = 0; i < num_grib_types; i++)
{
    grib_type[i] = NULL;
}

And lots of places where the scope of things can be reduced. e.g.

cppcheck --enable=all --std=c99 --force --inconclusive coda-grib.c
Checking coda-grib.c ...
coda-grib.c:2165:5: style: Assignment of function parameter has no effect outside the function. [uselessAssignmentArg]
    file_offset += 4;
    ^
coda-grib.c:1549:13: style: The scope of the variable 'intvalue' can be reduced. [variableScope]
    int32_t intvalue;
            ^
coda-grib.c:1681:26: style: The scope of the variable 'gds' can be reduced. [variableScope]
        coda_mem_record *gds;
                         ^
coda-grib.c:2316:22: style: The scope of the variable 'raw_data' can be reduced. [variableScope]
            uint8_t *raw_data;
                     ^
[SNIP]
svniemeijer commented 4 years ago

Are original approach was to aim for the highest compatibility with the code base and using C89 conventions was a conscious choice for that. This code style convention has stuck and I don't see a strong reason to change it. (besides that I personally also find code more readable with explicit variable declarations at the start of a block). If we move to C99 then we would need to change all code (to arrive again at a consistent coding style). This would be quite some effort and has the potential of introducing new bugs. So I am afraid that we have to decline this request.

schwehr commented 4 years ago

I strongly disagree with your assessment. I'll still send the pull request demonstrating what I mean, so it is there for the record. You can just close it.