more fixes #2508

neheb · 2023-02-11T02:28:14Z

No description provided.

ghost · 2023-02-11T02:29:34Z

👇 Click on the image for a new way to code review

Make big changes easier — review code in small groups of related files
Know where to start — see the whole change at a glance
Take a code tour — explore the change with an interactive tour
Make comments and review — all fully sync’ed with github

Try it now!

Legend

codecov · 2023-02-11T02:38:27Z

Codecov Report

Merging #2508 (2722e00) into main (976dcd8) will decrease coverage by 0.02%.
The diff coverage is 70.00%.

@@            Coverage Diff             @@
##             main    #2508      +/-   ##
==========================================
- Coverage   64.67%   64.66%   -0.02%     
==========================================
  Files         104      104              
  Lines       22256    22246      -10     
  Branches    10863    10858       -5     
==========================================
- Hits        14394    14385       -9     
- Misses       5619     5620       +1     
+ Partials     2243     2241       -2

Impacted Files	Coverage Δ
src/http.cpp	`0.00% <0.00%> (ø)`
src/image.cpp	`70.98% <0.00%> (ø)`
src/version.cpp	`89.01% <ø> (ø)`
src/preview.cpp	`47.65% <75.00%> (-0.45%)`	⬇️
app/exiv2.cpp	`63.32% <100.00%> (ø)`
src/asfvideo.cpp	`55.60% <100.00%> (-0.53%)`	⬇️
src/crwimage_int.cpp	`79.42% <100.00%> (ø)`
src/futils.cpp	`75.29% <100.00%> (ø)`
src/iptc.cpp	`65.20% <100.00%> (ø)`
src/pentaxmn_int.cpp	`72.72% <100.00%> (+0.38%)`	⬆️
... and 2 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

src/crwimage_int.cpp

app/actions.cpp

Pull request has been modified.

src/preview.cpp

Found with MSVC's C4244 Signed-off-by: Rosen Penev <[email protected]>

Signed-off-by: Rosen Penev <[email protected]>

Found with cppcheck. Signed-off-by: Rosen Penev <[email protected]>

cppcheck warns on localtime which is not necessarily threadsafe. Signed-off-by: Rosen Penev <[email protected]>

Signed-off-by: Rosen Penev <[email protected]>

Using the constructor is one less line. Plus it seems to fix bugs in Apple's clang. Signed-off-by: Rosen Penev <[email protected]>

Signed-off-by: Rosen Penev <[email protected]>

shorter Signed-off-by: Rosen Penev <[email protected]>

neheb · 2023-02-12T03:07:52Z

Remaining warnings. Not too sure about them.

2023-02-12T02:40:03.8824101Z D:\a\exiv2\exiv2\src\basicio.cpp(684) : warning C6308: 'realloc' might return null pointer: assigning null pointer to 'data_', which is passed as an argument to 'realloc', will cause the original memory block to be leaked.
2023-02-12T02:40:03.8824996Z D:\a\exiv2\exiv2\src\basicio.cpp(941) : warning C6262: Function uses '103332' bytes of stack.  Consider moving some data to heap.
2023-02-12T02:40:10.1230140Z D:\a\exiv2\exiv2\src\basicio.cpp(412) : warning C4701: potentially uninitialized local variable 'buf2' used
2023-02-12T02:41:00.8113062Z ../src/http.cpp(212): warning C4244: 'argument': conversion from 'int' to 'u_short', possible loss of data
2023-02-12T02:41:00.9206261Z D:\a\exiv2\exiv2\src\http.cpp(133) : warning C6262: Function uses '33996' bytes of stack.  Consider moving some data to heap.
2023-02-12T02:41:01.0155752Z D:\a\exiv2\exiv2\src\http.cpp(151) : warning C6031: Return value ignored: 'WSAStartup'.
2023-02-12T02:41:08.0309475Z D:\a\exiv2\exiv2\src\http.cpp(240) : warning C6386: Buffer overrun while writing to 'buffer'.: Lines: 239, 134, 136, 138, 140, 143, 144, 145, 150, 151, 154, 155, 156, 157, 158, 159, 161, 162, 163, 166, 167, 168, 169, 170, 171, 172, 175, 176, 177, 178, 179, 180, 181, 184, 187, 192, 194, 199, 200, 204, 207, 208, 210, 211, 212, 216, 225, 229, 230, 234, 235, 239, 240
2023-02-12T02:41:39.5074921Z C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.34.31933\include\xutility(4117): warning C4244: '=': conversion from 'uint32_t' to 'Exiv2::byte', possible loss of data
2023-02-12T02:42:00.5230007Z D:\a\exiv2\exiv2\src\properties.cpp(5178) : warning C6031: Return value ignored: 'std::unique_ptr<Exiv2::XmpKey::Impl,std::default_delete<Exiv2::XmpKey::Impl> >::->'.
2023-02-12T02:42:00.6012957Z D:\a\exiv2\exiv2\src\properties.cpp(5182) : warning C6031: Return value ignored: 'std::unique_ptr<Exiv2::XmpKey::Impl,std::default_delete<Exiv2::XmpKey::Impl> >::->'.
2023-02-12T02:42:18.7971400Z D:\a\exiv2\exiv2\src\quicktimevideo.cpp(849) : warning C6011: Dereferencing NULL pointer 'tv'. : Lines: 798, 799, 800, 802, 803, 804, 805, 807, 808, 809, 810, 811, 813, 814, 816, 818, 820, 822, 825, 828, 831, 835, 840, 841, 842, 843, 844, 846, 849
2023-02-12T02:42:19.0981220Z C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.34.31933\include\utility(172): warning C4244: 'initializing': conversion from '_Ty' to '_Ty1', possible loss of data
2023-02-12T02:42:50.8094181Z D:\a\exiv2\exiv2\src\version.cpp(220) : warning C6240: (<expression> && <non-zero constant>) always evaluates to the result of <expression>:  Did you intend to use the bitwise-and (&) operator? If not, consider removing the redundant '<non-zero constant>' and the && operator.
2023-02-12T02:42:50.8100266Z D:\a\exiv2\exiv2\src\version.cpp(222) : warning C6240: (<expression> && <non-zero constant>) always evaluates to the result of <expression>:  Did you intend to use the bitwise-and (&) operator? If not, consider removing the redundant '<non-zero constant>' and the && operator.
2023-02-12T02:42:51.0751153Z D:\a\exiv2\exiv2\src\version.cpp(220) : warning C6326: Potential comparison of a constant with another constant.
2023-02-12T02:43:00.4190440Z D:\a\exiv2\exiv2\src\version.cpp(222) : warning C6326: Potential comparison of a constant with another constant.
2023-02-12T02:43:15.7764035Z D:\a\exiv2\exiv2\src\xmpsidecar.cpp(39) : warning C6262: Function uses '65700' bytes of stack.  Consider moving some data to heap.
2023-02-12T02:44:28.3502423Z D:\a\exiv2\exiv2\src\pentaxmn_int.cpp(909) : warning C6297: Arithmetic overflow.  Results might not be an expected value.

kevinbackhouse · 2023-02-13T09:56:08Z

src/image.cpp

-            DataBuf bytes(byteslen);                                                  // allocate a buffer
-            io.readOrThrow(bytes.data(), byteslen, ErrorCode::kerCorruptedMetadata);  // read
-            MemIo memIo(bytes.c_data(), byteslen);                                    // create a file
+            auto b = DataBuf(byteslen);                                           // allocate a buffer


What's the benefit of doing it this way?

IIRC bytes it a shadowed declaration. because of copy elision, it's the same.

kevinbackhouse · 2023-02-13T09:57:32Z

src/preview.cpp

-    if (decodeBase64Table[static_cast<unsigned char>(src[srcPos])] != invalid)
-      validSrcSize++;
-  }
+  auto validSrcSize = static_cast<unsigned long>(


Can we use size_t here? I'd like to eliminate long from the codebase.

neheb force-pushed the mf branch from 25bfe3d to 6ca3c6b Compare February 11, 2023 06:20

piponazo mentioned this pull request Feb 11, 2023

meson CI additions #2505

Merged

piponazo previously approved these changes Feb 11, 2023

View reviewed changes

src/crwimage_int.cpp Outdated Show resolved Hide resolved

app/actions.cpp Outdated Show resolved Hide resolved

neheb force-pushed the mf branch from 6ca3c6b to ebc2d6e Compare February 11, 2023 22:42

neheb force-pushed the mf branch 4 times, most recently from d0daf70 to 76a6e7f Compare February 12, 2023 01:22

github-advanced-security bot found potential problems Feb 12, 2023

View reviewed changes

src/preview.cpp Fixed Show fixed Hide fixed

neheb force-pushed the mf branch 5 times, most recently from 3b19df2 to 355b626 Compare February 12, 2023 02:20

github-advanced-security bot found potential problems Feb 12, 2023

View reviewed changes

src/preview.cpp Fixed Show fixed Hide fixed

neheb force-pushed the mf branch from 355b626 to 5a7bbef Compare February 12, 2023 02:52

neheb added 9 commits February 11, 2023 19:04

make conversions explicit

c91a51f

Found with MSVC's C4244 Signed-off-by: Rosen Penev <[email protected]>

use upper() function

412fd7f

Signed-off-by: Rosen Penev <[email protected]>

fix memory leak

8c1e79d

Found with cppcheck. Signed-off-by: Rosen Penev <[email protected]>

replace localtime with _s/_r variant

4748c8b

cppcheck warns on localtime which is not necessarily threadsafe. Signed-off-by: Rosen Penev <[email protected]>

explicit conversion

38e4dbe

Signed-off-by: Rosen Penev <[email protected]>

fix wrong snprintf format

4c36e2e

Signed-off-by: Rosen Penev <[email protected]>

replace array with vector

fadade3

Using the constructor is one less line. Plus it seems to fix bugs in Apple's clang. Signed-off-by: Rosen Penev <[email protected]>

use count_if

f921e88

Signed-off-by: Rosen Penev <[email protected]>

use std::replace

2722e00

shorter Signed-off-by: Rosen Penev <[email protected]>

neheb force-pushed the mf branch from 5a7bbef to 2722e00 Compare February 12, 2023 03:04

kevinbackhouse reviewed Feb 13, 2023

View reviewed changes

kevinbackhouse approved these changes Feb 13, 2023

View reviewed changes

neheb merged commit d7bf726 into Exiv2:main Feb 13, 2023

neheb deleted the mf branch February 13, 2023 17:35

nehaljwani mentioned this pull request May 8, 2023

Is a new release happening soon? #2406

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

more fixes #2508

more fixes #2508

neheb commented Feb 11, 2023

ghost commented Feb 11, 2023 •

edited by ghost

Loading

codecov bot commented Feb 11, 2023 •

edited

Loading

neheb commented Feb 12, 2023

kevinbackhouse Feb 13, 2023

neheb Feb 13, 2023

kevinbackhouse Feb 13, 2023

more fixes #2508

more fixes #2508

Conversation

neheb commented Feb 11, 2023

ghost commented Feb 11, 2023 • edited by ghost Loading

Legend

codecov bot commented Feb 11, 2023 • edited Loading

Codecov Report

neheb commented Feb 12, 2023

kevinbackhouse Feb 13, 2023

Choose a reason for hiding this comment

neheb Feb 13, 2023

Choose a reason for hiding this comment

kevinbackhouse Feb 13, 2023

Choose a reason for hiding this comment

ghost commented Feb 11, 2023 •

edited by ghost

Loading

codecov bot commented Feb 11, 2023 •

edited

Loading