Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[¿Bug?] A couple of compile warnings may denote coding flaws #27

Open
Olf0 opened this issue Mar 6, 2022 · 7 comments
Open

[¿Bug?] A couple of compile warnings may denote coding flaws #27

Olf0 opened this issue Mar 6, 2022 · 7 comments
Labels
enhancement Enhances something help wanted Extra attention is needed

Comments

@Olf0
Copy link
Collaborator

Olf0 commented Mar 6, 2022

  1. unused parameter
src/search.cpp: In member function 'void Search::shareFile(const QString&)':
src/search.cpp:289:39: warning: unused parameter 'file' [-Wunused-parameter]
 void Search::shareFile(const QString &file)
                        ~~~~~~~~~~~~~~~^~~~
  1. unused parameter
src/dropboxclient.cpp: In member function 'void DropboxClient::moveFile(QString, QString)':
src/dropboxclient.cpp:697:54: warning: unused parameter 'dest' [-Wunused-parameter]
 void DropboxClient::moveFile(QString source, QString dest)
                                              ~~~~~~~~^~~~
  1. enumeration value 'INVALID' not handled
src/dropbox/dropbox.cpp: In member function 'QUrl Dropbox::apiToUrl(Dropbox::Api)':
src/dropbox/dropbox.cpp:35:11: warning: enumeration value 'INVALID' not handled in switch [-Wswitch]
     switch(api)
           ^
  1. unused result
src/compressedfiles.cpp:92:11: warning: ignoring return value of 'int system(const char*)', declared with attribute warn_unused_result [-Wunused-result]
     system(ttt1.toUtf8());
     ~~~~~~^~~~~~~~~~~~~~~
@Olf0
Copy link
Collaborator Author

Olf0 commented Sep 17, 2023

@nephros, I am trying to revive FileCase and would appreciate very much, if you look at these.

If the code is too convoluted to comprehend how to resolve one or multiple of these warnings within appropriate time, ultimately "it is just a bunch of warnings".

@nephros
Copy link

nephros commented Sep 20, 2023

  1. unused parameter
src/search.cpp: In member function 'void Search::shareFile(const QString&)':
src/search.cpp:289:39: warning: unused parameter 'file' [-Wunused-parameter]
 void Search::shareFile(const QString &file)
                        ~~~~~~~~~~~~~~~^~~~

https://github.com/sailfishos-applications/filecase/blob/devel/src/search.cpp#L289

Well, it's an unused parameter. void Search::shareFile(const QString &file) takes a file argument, but the code does not use it. This is due to the commented-out section in lines 297ff, where it used to be used.

To "fix" that warning you can add some noop that uses the file parameter, like if (file == "") { true } or if 0 { QString dummy = file } or so Q_UNUSED, see comment below.
But really, it's not worth any changes IMO.

@nephros
Copy link

nephros commented Sep 20, 2023

2. unused parameter
src/dropboxclient.cpp: In member function 'void DropboxClient::moveFile(QString, QString)':
src/dropboxclient.cpp:697:54: warning: unused parameter 'dest' [-Wunused-parameter]
 void DropboxClient::moveFile(QString source, QString dest)
                                              ~~~~~~~~^~~~

Well, dest is not used, the code always uses d->path and the file name of the source parameter as the destination for the copy operation. It looks like (from using GitHubs "search for this symbol" function") that this is called from nowhere.
If that is true, this code could be removed. Alternatively, remove the dest parameter from DropboxClient::moveFile in both the .cpp and the header file.

@nephros
Copy link

nephros commented Sep 20, 2023

3. enumeration value 'INVALID' not handled
src/dropbox/dropbox.cpp: In member function 'QUrl Dropbox::apiToUrl(Dropbox::Api)':
src/dropbox/dropbox.cpp:35:11: warning: enumeration value 'INVALID' not handled in switch [-Wswitch]
     switch(api)
           ^

Dropbox::Api is defined here:

and includes a value called INVALID.

This is not handled in the switch statement at

switch(api)

The Warning could be fixed by handling the INVALID case in the switch (maybe log a warning or so). But I haven't studied the code to say whether the omission was intentional.

@nephros
Copy link

nephros commented Sep 20, 2023

4. unused result
src/compressedfiles.cpp:92:11: warning: ignoring return value of 'int system(const char*)', declared with attribute warn_unused_result [-Wunused-result]
     system(ttt1.toUtf8());
     ~~~~~~^~~~~~~~~~~~~~~

Meh, totally harmless. system returns something which nobody needs.

Trivial fix by doing something like

 (void) system(ttt1.toUtf8());

(I love casting things into the void.)

Although running an external command should probably be handled in a more advanced fashion than just firing the external command and hoping it succeeds.

@nephros
Copy link

nephros commented Sep 20, 2023

Addendum to the unused parms:

There is also the Qt Q_UNUSED() macro which can be used to wrap such cases.

@Olf0
Copy link
Collaborator Author

Olf0 commented Sep 23, 2023

@nephros, thank you, your feedback is much appreciated!

It was exactly what I wanted, both in form and content:

  • Form
    A quick analysis, tersely denoting for each if this is a significant issue and indicating a route how it may be resolved.
  • Content
    Luckily none of these warnings turned out to indicate something serious and the brief guidance how I might resolve two of these on my own is really helpful.

Consequently I plan to address higher priority tasks first (e.g. Transifex integration), then take care about the two issues I should be able to rectify easily by the help of your guidance and lastly I may come back to you for addressing the final ones (sure not this year).

@Olf0 Olf0 added enhancement Enhances something help wanted Extra attention is needed labels Nov 16, 2023
@Olf0 Olf0 changed the title A couple of compile warnings denote coding flaws [¿Bug?] A couple of compile warnings denote coding flaws Feb 7, 2024
@Olf0 Olf0 changed the title [¿Bug?] A couple of compile warnings denote coding flaws [¿Bug?] A couple of compile warnings may denote coding flaws Feb 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhances something help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants