CLang warnings for DCMTK development master

[chaircrusher]

Excuse me for re-posting this on the Installation board, maybe it's more appropriate here than on 'general'

While the resulting libraries seem to work fine, there are a bunch of warnings you get if you build DCMTK with CLang.

1. There's a problem with 8-bit ASCII characters > 127 Decimal. In particular in the tests there are statements like

Code: Select all

  CHECK_GOOD( "LT-01", DcmLongText::checkStringValue(" Hallo \\ 12345 \\ äöüß ") )

in dcmdata/tests/tchval.cc which contain characters with umlauts in line. 8 bit ASCII in source code is problematic; ISO Latin-1 is the most common representation, but it isn't the only one. Replacing the accented characters with their equivalent octal codes can silence the warning.

2. dcmnet/libsrc/dulfsm.cc line 928:

Code: Select all

           (void) memset(userPresentationCtx, 0, sizeof(userPresentationCtx));

I think what is intended is this:

Code: Select all

          (void) memset(userPresentationCtx, 0, sizeof(*userPresentationCtx));

The warning is:

DCMTK/dcmnet/libsrc/dulfsm.cc:928:58: warning: 'memset' call operates on
objects of type 'DUL_PRESENTATIONCONTEXT' while the size is based on a different type
'DUL_PRESENTATIONCONTEXT *' [-Wsizeof-pointer-memaccess]
(void) memset(userPresentationCtx, 0, sizeof(userPresentationCtx));
~~~~~~~~~~~~~~~~~~~ ^~~~~~~~~~~~~~~~~~~
DCMTK/dcmnet/libsrc/dulfsm.cc:928:58: note: did you mean to dereference
the argument to 'sizeof' (and multiply it by the number of elements)?
(void) memset(userPresentationCtx, 0, sizeof(userPresentationCtx));

3.There's a couple of style issues (see patch below) that cause warnings; CLang warns of conditionals like
if(condition);
because that can produce misleading results. Putting the semicolon on its own line silences the warning. Our coding standard is to always use curly braces on conditionals. Otherwise you can end up with coding errors based on visually misleading source code. See patch at bottom of file.

4. For the CMake build, building apps and tests should be optional. I build DCMTK for the library, and the tests and executables are not properly part of a project that uses the DCMTK libraries.

Code: Select all

diff --git a/dcmdata/apps/dump2dcm.cc b/dcmdata/apps/dump2dcm.cc
index 069ccf8..c647320 100644
--- a/dcmdata/apps/dump2dcm.cc
+++ b/dcmdata/apps/dump2dcm.cc
@@ -223,8 +223,10 @@ isaCommentLine(const char *s)
     OFBool isComment = OFFalse; /* assumption */
     int len = strlen(s);
     int i = 0;
-    for (i = 0; i < len && isspace(TO_UCHAR(s[i])); i++) /*loop*/;
-        isComment = (s[i] == DCM_DumpCommentChar);
+    for (i = 0; i < len && isspace(TO_UCHAR(s[i])); i++)
+      /*loop*/;
+
+    isComment = (s[i] == DCM_DumpCommentChar);
     return isComment;
 }
 
diff --git a/dcmdata/libi2d/i2djpgs.cc b/dcmdata/libi2d/i2djpgs.cc
index 3208ad0..6bddd3b 100644
--- a/dcmdata/libi2d/i2djpgs.cc
+++ b/dcmdata/libi2d/i2djpgs.cc
@@ -413,7 +413,7 @@ OFCondition I2DJpegSource::copyJPEGStream(char*& pixelData,
   while (entry != m_jpegFileMap.end())
   {
     marker = (*entry)->marker;
-    if ( (marker == E_JPGMARKER_APP0) )
+    if ( marker == E_JPGMARKER_APP0 )
     {
       DCMDATA_LIBI2D_DEBUG("I2DJpegSource: Skipping application segment APP0");
       bytePosJFIF = (*entry)->bytePos - 1; // include first byte of marker (FF)
diff --git a/dcmjpls/libcharls/scan.h b/dcmjpls/libcharls/scan.h
index f51597d..4a743b1 100644
--- a/dcmjpls/libcharls/scan.h
+++ b/dcmjpls/libcharls/scan.h
@@ -779,7 +779,7 @@ ProcessLine* JlsCodec<TRAITS,STRATEGY>::CreateProcess(void* pvoidOut)
 	if (Info().colorTransform == 0)
 		return new ProcessTransformed<TransformNone<OFTypename TRAITS::SAMPLE> >(pvoidOut, Info(), TransformNone<SAMPLE>());
 
-	if ((Info().bitspersample == sizeof(SAMPLE)*8))
+	if ( Info().bitspersample == sizeof(SAMPLE)*8 )
 	{
 		switch(Info().colorTransform)
 		{
diff --git a/dcmqrdb/libsrc/dcmqrdbi.cc b/dcmqrdb/libsrc/dcmqrdbi.cc
index fc7982b..ad2bba6 100644
--- a/dcmqrdb/libsrc/dcmqrdbi.cc
+++ b/dcmqrdb/libsrc/dcmqrdbi.cc
@@ -840,8 +840,9 @@ static void DB_RemoveEnclosingSpaces (char *string)
     /** Ship trailing spaces
      */
 
-    for (pc2 = string + strlen (string) - 1; *pc2 == ' '; pc2--);
-        pc2++;
+    for (pc2 = string + strlen (string) - 1; *pc2 == ' '; pc2--)
+      /*loop*/;
+    pc2++;
     *pc2 = '\0';
 }

[J. Riesmeier]

See this: viewtopic.php?f=1&t=3609