rgamble / libcsv

Fast and flexible CSV library written in pure ANSI C that can read and write CSV data.
GNU Lesser General Public License v2.1
181 stars 40 forks source link

configure file needs exec permissions #1

Closed bobhairgrove closed 7 years ago

bobhairgrove commented 7 years ago

Hi Robert,

Congratulations on moving this to GitHub! Hope this little gem will find a larger audience now.

Two little issues:

  1. The "configure" file seems to be missing executable permissions (on *nix systems);
  2. There is an unused variable warning from gcc:
    libcsv.c: In function ‘csv_fini’:
    libcsv.c:163:7: warning: variable ‘pstate’ set but not used [-Wunused-but-set-variable]
    int pstate = p->pstate;

If you could fix this, it will compile without any warnings. Go to line 171 in libcsv.c and change this line:

  if (p->pstate == FIELD_BEGUN && p->quoted && p->options & CSV_STRICT && p->options & CSV_STRICT_FINI) {

to this:

  if (pstate == FIELD_BEGUN && p->quoted && p->options & CSV_STRICT && p->options & CSV_STRICT_FINI) {

Alternatively, you could just remove line 163 since pstate apparently isn't used anywhere else.

I haven't tested the examples yet, but running "make check" seems to work OK.

Cheers, Bob

rgamble commented 7 years ago

Thanks for the feedback. I think the pstate variable might be needed by the macros used in the function but I'll take a closer look and run everything through PC-lint again when I get a chance.

On Sat, Dec 10, 2016 at 5:45 AM, bobhairgrove notifications@github.com wrote:

Hi Robert,

Congratulations on moving this to GitHub! Hope this little gem will find a larger audience now.

Two little issues:

  1. The "configure" file seems to be missing executable permissions (on *nix systems);
  2. There is an unused variable warning from gcc:

libcsv.c: In function ‘csv_fini’: libcsv.c:163:7: warning: variable ‘pstate’ set but not used [-Wunused-but-set-variable] int pstate = p->pstate;

If you could fix this, it will compile without any warnings. Go to line 171 in libcsv.c and change this line:

if (p->pstate == FIELD_BEGUN && p->quoted && p->options & CSV_STRICT && p->options & CSV_STRICT_FINI) {

to this:

if (pstate == FIELD_BEGUN && p->quoted && p->options & CSV_STRICT && p->options & CSV_STRICT_FINI) {

I haven't tested the examples yet, but running "make check" seems to work OK.

Cheers, Bob

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/rgamble/libcsv/issues/1, or mute the thread https://github.com/notifications/unsubscribe-auth/AArFyKwfDXypyTjpbDXCccc8tJ6CR4uHks5rGoLMgaJpZM4LJnku .

-- Thanks,

Robert Gamble http://www.robertgamble.net/

bobhairgrove commented 7 years ago

Hi Robert,

I ran it through the preprocessor by doing gcc -E -o libcsv_pre.c libcsv.c and compared the preprocessed source with the original. Indeed, pstate is set several times by code generated by the macros, so just removing line 163 isn't the best option. However, its value is never read, AFAICT.

FWIW, the warning goes away when I compile the resulting preprocessed source. Not sure why, though.

bobhairgrove commented 7 years ago

Thanks for fixing the file permissions on configure. I'm opening a new issue for -Wunused-but-set-variable and closing this one.