Compiling with -Wall shows a large amount of warnings which should be fixed

[heckflosse]

I want to try to make rt a bit more -Wall-clean. I will start with compilation units in rtengine Checked means it's clean. Unchecked means it's not clean or not tested yet.

• [x] colortemp.cc
• [x] curves.cc
• [x] flatcurves.cc
• [x] diagonalcurves.cc
• [x] dcraw.cc (fixed some warnings and disabled the remaining ones)
• [x] iccstore.cc
• [x] color.cc
• [x] dfmanager.cc
• [x] ffmanager.cc
• [x] gauss.cc
• [x] rawimage.cc
• [x] image8.cc
• [x] image16.cc
• [x] imagefloat.cc
• [x] imagedata.cc
• [x] imageio.cc
• [x] improcfun.cc
• [x] init.cc
• [x] dcrop.cc (Ingo)
• [x] loadinitial.cc
• [x] procparams.cc
• [x] rawimagesource.cc (Ingo)
• [x] demosaic_algos.cc (Ingo)
• [x] shmap.cc (Ingo)
• [x] simpleprocess.cc (Ingo)
• [x] refreshmap.cc
• [x] fast_demo.cc (Ingo)
• [x] amaze_demosaic_RT.cc (Ingo)
• [x] CA_correct_RT.cc (Ingo)
• [x] cfa_linedn_RT.cc (Ingo)
• [x] green_equil_RT.cc (Ingo)
• [x] hilite_recon.cc (Ingo)
• [x] expo_before_b.cc
• [x] stdimagesource.cc (nothing)
• [x] myfile.cc (Ingo)
• [x] iccjpeg.cc (nothing)
• [x] improccoordinator.cc (Ingo)
• [x] pipettebuffer.cc (Flössie)
• [x] coord.cc (Flössie)
• [x] processingjob.cc (Flössie)
• [x] rtthumbnail.cc (Flössie)
• [x] utils.cc (jacq)
• [x] labimage.cc (jacq)
• [x] slicer.cc (jacq)
• [x] cieimage.cc (jacq)
• [x] iplab2rgb.cc (jacq)
• [x] ipsharpen.cc (jacq)
• [x] iptransform.cc (jacq)
• [x] ipresize.cc (jacq)
• [X] ipvibrance.cc (jacq)
• [x] icons.cc (jacq)
• [x] imagedimensions.cc (jacq)
• [x] jpeg_ijg/jpeg_memsrc.cc
• [x] jdatasrc.cc (jacq)
• [x] iimage.cc (jacq)
• [x] EdgePreservingDecomposition.cc (jacq)
• [x] cplx_wavelet_dec.cc (jacq)
• [x] FTblockDN.cc (jacq)
• [x] PF_correct_RT.cc (jacq)
• [x] previewimage.cc (jacq)
• [x] ipwavelet.cc (jacq)
• [x] dirpyr_equalizer.cc
• [x] calc_distort.cc
• [x] lcp.cc
• [x] dcp.cc
• [x] ipretinex.cc
• [x] cJSON.c
• [x] camconst.cc
• [x] klt/convolve.cc
• [x] klt/error.cc
• [x] klt/klt.cc
• [x] klt/klt_util.cc
• [x] klt/pnmio.cc
• [x] klt/pyramid.cc
• [x] klt/selectGoodFeatures.cc
• [x] klt/storeFeatures.cc
• [x] klt/trackFeatures.cc
• [x] klt/writeFeatures.cc
• [x] clutstore.cc
• [x] ciecam02.cc (Ingo)
• [x] pixelshift.cc

[Floessie]

@heckflosse I don't see a branch, yet. I could join you from time to time.

[heckflosse]

@Floessie I will create a branch now after committing fixes for the compilation units at top of the list. Maybe we should agree about the procedure to avoid double work. Kind of I start from top and you start from bottom until we meet (or vice versa)

[heckflosse]

@Floessie I created the new branch wallfix ;-)

[Floessie]

Kind of I start from top and you start from bottom until we meet

:+1:

[Desmis]

Can I help, even Ingo start from top, and Floessie start from bottom :)

[Floessie]

@Desmis Maybe you could pick the ones you like to work on now from the end of the list and put your name on it. Then I'll do the same later for the ones that I'm going to clean.

[Desmis]

@Floessie Ok, but how to do, I am a not at all a specialist for compilation : a) for compilation directives with windows , which files in cmake ? b) for only have warning for one specific file c) etc. Thank you Jacques

[heckflosse]

@Desmis For temporary usage of compiler switches I always cheat by modifying CMakeCache.txt in build folder. HowTo:

1. Open CMakeCache.txt
2. Search for PROC_TARGET_2_FLAGS:STRING=-march=native
3. Add - Wall to that line and save
4. make, but ignore all the warnings because it makes a full build now
5. open a source file from the list above (for example ipwavelet.cc) and add space in first line, delete that space and save
6. make, now you get only the warnings related to that file and the included files.

[Desmis]

Ingo

OK, thank you, I'll try soon :)

[Desmis]

Ok, thats works :) I just change ipvibrance.cc in the middle of the list jacques

[heckflosse]

@Desmis Jacques, please do not reformat the code during this cleanup. It makes it impossible to see what you fixed.

[Desmis]

Ok :)

[Floessie]

BTW: What's the matter of adding spaces between function name and the opening parenthesis? Haven't seen it very often, but there must be some reasoning to it.

PS: I'll join you on the weekend.

[heckflosse]

@Desmis Jacques, I will take improcfun.cc now :)

[Desmis]

@heckflosse How do we do, for commit and push... I am afraid of probable conflicts, but maybe there are no risks ?

[heckflosse]

@Desmis One solution could be to use wallfix_desmis, wallfix_flossie and wallfix_ingo branches

Edit: Btw. I just pushed

[heckflosse]

@Floessie @Desmis I will work now on wallfix_ingo branch to avoid conflicts. I always make one commit per compilation unit

[heckflosse]

@Floessie @Desmis Scratch my lost post. I have to think about that first

[Beep6581]

I don't know how messy this will get, but if you are able to confine the changes to individual files then working in the same branch and committing/pulling every time you're done with a single file would be easy and it would make this easier for others to checkout and test.

[Desmis]

I create wall_jacq and push changes for FTblockdn.cc Edgeprervingdecomposition.cc previewimage.cc with changes and Pfcorrect.cc iimage.cc jdatsrc.cc imagedimensions.cc icons.cc utils.cc - no nead changes

Now I make a pause,To be sure we work well together :)

[heckflosse]

@Desmis I will take rawimagesource.cc, demosaic_algos.cc and fast_demo.cc now @Beep6581 I made a commit for each single compilation unit and from now on I will also make a push for each commit

[Floessie]

@heckflosse I marked my initial three and will push to your branch.

[heckflosse]

@Floessie Oups, saw this too late. I hope my commit from now will not conflict.

[Floessie]

@heckflosse Sure it will, but I'll revert mine. :smile:

[heckflosse]

@Floessie @Desmis I will take improccoordinator.cc, simpleprocess.cc and dcrop.cc now

[Floessie]

@heckflosse How comes, there are files you listed that don't trigger here?

[heckflosse]

@Floessie You mean in my commits? In first post I just listed all compilation units of rtengine

[Floessie]

@heckflosse No, I mean, I compile them with -Wall, but don't get a warning...

[Floessie]

@heckflosse

In first post I just listed all compilation units of rtengine

Ah, that is why.

[Floessie]

@heckflosse Sorry, no time left tonight. I was working on rtthumbnail.cc: If we have a height or width, or a file size, shall we turn that into an unsigned or just cast the place with the comparison to signed?

[heckflosse]

@Floessie @Desmis It's really great to see the progress of this task. I expected that to take much longer. Now I think we can at least make rtengine compilation units -Wall-clean very soon (except dcraw.cc maybe...)

[heckflosse]

@Floessie I use unsigned when there is no subtraction. When there is a subtraction I use signed because I'm too lazy to search for possible side effects.

[heckflosse]

@Floessie Can I push some changes or do you have something on hold?

[heckflosse]

@Floessie I pushed my changes @Desmis I also merged your changes into wallfix branch

[agriggio]

sorry to jump in this late, but if you need help let me know! also, if you care usually clang is much more pedantic than GCC with -Wall

[heckflosse]

@agriggio the rtengine part is almost done now (at least for -Wall). But helping out for rtgui compilation units and also checking using more pedantic clang would be great :+1:

[heckflosse]

I also just started a cppcheck run...

[agriggio]

OK, I'll see what I can do

[heckflosse]

@agriggio Great :+1:

[heckflosse]

@Floessie @Desmis @agriggio @Beep6581 Shall I open a new checklist for rtgui or add it to this issue?

[Desmis]

@heckflosse Ingo I prefer a new checklist :)

[Desmis]

@heckflosse I have begun rtgui

No problem with : wavelet.cc, vibrance.cc One small change to whitebalance.cc

[Desmis]

One change to colortoning.cc

No problem with retinex.cc blackwhite.cc colorappearance.cc

[Desmis]

No problem with : dirpyrdenoise.cc dirpyrequalizer.cc labcurve.cc rawexposure.cc tonecurve.cc sharpening.cc sharpenedge.cc rawcacorrection.cc prsharpening.cc icmpanel.cc

[heckflosse]

Here's the checklist for rtgui:

• [x] editwindow.cc
• [x] batchtoolpanelcoord.cc
• [x] paramsedited.cc
• [x] cropwindow.cc
• [x] previewhandler.cc
• [x] previewwindow.cc
• [x] navigator.cc
• [x] indclippedpanel.cc
• [x] previewmodepanel.cc
• [x] filterpanel.cc
• [x] exportpanel.cc
• [x] cursormanager.cc
• [x] rtwindow.cc
• [x] renamedlg.cc
• [x] recentbrowser.cc
• [x] placesbrowser.cc
• [x] filepanel.cc
• [x] editorpanel.cc
• [x] batchqueuepanel.cc
• [x] checkbox.cc
• [x] ilabel.cc
• [x] thumbbrowserbase.cc
• [x] adjuster.cc
• [x] filebrowserentry.cc
• [x] filebrowser.cc
• [x] filethumbnailbuttonset.cc
• [x] cachemanager.cc
• [x] cacheimagedata.cc
• [x] shcselector.cc
• [x] perspective.cc
• [x] thresholdselector.cc
• [x] thresholdadjuster.cc
• [x] clipboard.cc
• [x] thumbimageupdater.cc
• [x] bqentryupdater.cc
• [x] lensgeom.cc
• [x] coloredbar.cc
• [x] edit.cc
• [x] coordinateadjuster.cc
• [x] coarsepanel.cc
• [x] cacorrection.cc
• [x] chmixer.cc
• [x] blackwhite.cc
• [x] resize.cc
• [x] icmpanel.cc
• [x] crop.cc
• [x] shadowshighlights.cc
• [x] impulsedenoise.cc
• [x] dirpyrdenoise.cc
• [x] epd.cc
• [x] exifpanel.cc
• [x] toolpanel.cc
• [x] lensprofile.cc
• [x] lockablecolorpicker.cc
• [x] sharpening.cc
• [x] vibrance.cc
• [x] rgbcurves.cc
• [x] colortoning.cc
• [x] whitebalance.cc
• [x] vignetting.cc
• [x] gradient.cc
• [x] pcvignette.cc
• [x] rotate.cc
• [x] distortion.cc
• [x] crophandler.cc
• [x] dirbrowser.cc
• [x] curveeditor.cc
• [x] curveeditorgroup.cc
• [x] diagonalcurveeditorsubgroup.cc (Ingo)
• [x] flatcurveeditorsubgroup.cc
• [x] filecatalog.cc
• [x] extprog.cc
• [x] previewloader.cc
• [x] rtimage.cc
• [x] inspector.cc
• [x] histogrampanel.cc
• [x] history.cc
• [x] imagearea.cc
• [x] imageareapanel.cc
• [x] iptcpanel.cc
• [x] labcurve.cc
• [x] main.cc (disabled -Wdeprecated-declarations)
• [x] multilangmgr.cc
• [x] mycurve.cc
• [x] myflatcurve.cc
• [x] mydiagonalcurve.cc
• [x] options.cc
• [x] retinex.cc
• [x] preferences.cc
• [x] profilepanel.cc
• [x] saveasdlg.cc
• [x] saveformatpanel.cc
• [x] soundman.cc
• [x] splash.cc
• [x] thumbnail.cc
• [x] tonecurve.cc
• [x] toolbar.cc
• [x] pathutils.cc
• [x] guiutils.cc
• [x] threadutils.cc
• [x] zoompanel.cc
• [x] toolpanelcoord.cc
• [x] thumbbrowserentrybase.cc
• [x] batchqueueentry.cc
• [x] batchqueue.cc
• [x] lwbutton.cc
• [x] lwbuttonset.cc
• [x] batchqueuebuttonset.cc
• [x] browserfilter.cc
• [x] exiffiltersettings.cc
• [x] profilestore.cc
• [x] partialpastedlg.cc
• [x] sensorbayer.cc
• [x] sensorxtrans.cc
• [x] preprocess.cc
• [x] bayerpreprocess.cc
• [x] bayerprocess.cc
• [x] bayerrawexposure.cc
• [x] xtransprocess.cc
• [x] xtransrawexposure.cc
• [x] darkframe.cc
• [x] flatfield.cc
• [x] rawcacorrection.cc
• [x] rawexposure.cc
• [x] wavelet.cc
• [x] dirpyrequalizer.cc
• [x] hsvequalizer.cc
• [x] defringe.cc
• [x] popupcommon.cc
• [x] popupbutton.cc
• [x] popuptogglebutton.cc
• [x] sharpenedge.cc
• [x] sharpenmicro.cc
• [x] colorappearance.cc
• [x] filmsimulation.cc
• [x] prsharpening.cc
• [x] dynamicprofile.cc
• [x] dynamicprofilepanel.cc

[agriggio]

Can I give a shot to the files in rtengine? Clang still gives me some meaningful warnings (I silenced the less useful ones). Also, what about rtexif? And why is rgengine/fujicompressed.cc not listed?

[heckflosse]

@agriggio Sure you can give it a shot :+1: rtengine/fujicompressed.cc is not listed because currently it's not an own compilation unit. But I agree that we should fix warnings in this one too! rtexif also needs to be checked but I wanted to have an own checklist for it too when rtgui is completed

[agriggio]

rtexif is almost clean already, there's only one warning - I can fix that.

[agriggio]

I'm also working on rtgui -- I'm basically done. I'm leaving the deprecation warnings about gdk_threads* in main.cc

[agriggio]

it seems I was wrong about rtexif. Clang gives no warning now, but gcc shows many places in which the return values of fread, read, fwrite and write are ignored. How should we fix those though? The easy solution is to cast explicitly to void, should we do that or should we try to handle the possible failures?