suggested clang patch: unused variable when compiling fastx_toolkit

[JeffreyDeanBrown]

I'm fairly new to suggesting fixes for code, but I think I've got a useful fix for an error that comes up when compiling the toolkit using clang 17. I haven't had this issue when compiling with gcc 11.4.0 on ubuntu, but my colleague had the issue with clang 17 on macOS.

There is an unused variable in the fastx_artifacts_filter code that throws an error, specifically "n_count". I used grep to make sure that variable is not used in any other code (it's not) and just deleted it using a sed script. Here is the script that fixes the issue, run this from the fastx_toolkit-0.0.14 directory:

$ sed -i '88,90d;58d' src/fastx_artifacts_filter/fastx_artifacts_filter.c

Edit (07/26/2024):

Now that I have read more troubleshooting posts online, I think I should have been more explicit with my troubleshooting process.

Specifically, I want to expand on

I used grep to make sure that variable is not used in any other code (it's not)

and say, explicitly, that the command:

grep -rw n_count

within the root directory of the file returns this:

src/fastx_artifacts_filter/fastx_artifacts_filter.c: int n_count=0; src/fastx_artifacts_filter/fastx_artifacts_filter.c: n_count++;
Therefore it appears to be a variable that is declared and incremented, but not used for anything else. I don't think there's going to be future updates for this project, so removing this variable is a viable fix and does not affect the functionality of the program in any way.

However, I think there is a more appropriate solution. I believe this error is simply an elevated "warning" using the -Werror option, which is on it's own harmless (more info here). removing the -Werror option may be more appropriate and prevent future issues with Clang updates, using the command:

sed -i '' 's/-Werror//' configure.ac

Though I no longer have access to a mac to verify this. I will try to verify this using Clang 17 when I have time

[amulyagarimella]

Thank you, this was helpful :) Tiny edit, I used sed -i '' '88,90d;58d' src/fastx_artifacts_filter/fastx_artifacts_filter.c since I was getting this error: sed: 1: "src/fastx_artifacts_fil ...": bad flag in substitute command: '/'

(as per this stackoverflow post)

[JeffreyDeanBrown]

Ah good catch, it looks like there's some funny differences between MacOS and Linux versions of sed, specifically with implicit definitions of backup files (very well explained on this stackoverflow post)

It also looks like the MacOS version that has been posted (with -i '' ) does not work with Linux systems. So, in case anyone is compiling with Clang on a Linux system and runs into this problem:

The MacOS sed command for deleting the variable is:
sed -i '' '88,90d;58d' src/fastx_artifacts_filter/fastx_artifacts_filter.c
and the Linux command is:
sed -i '88,90d;58d' src/fastx_artifacts_filter/fastx_artifacts_filter.c
and the MacOS sed to remove the -Werror is:
sed -i '' 's/-Werror//' configure.ac
and the Linux command is:
sed -i 's/-Werror//' configure.ac

it looks like there's also Linux/GNU versions of sed available for MacOS on brew (per the above link), that's also an option