WITH_ZCHUNK macro affects ABI #298
Comments
|
I forgot to notice that the biggest mistake of the current code is that compiled librepo has struct LrDownloadTarget of a certain size, but applications including librepo header files without matching WITH_ZCHUNK will see struct LrDownloadTarget of a different size. This can lead to memory corruptions and segfaults. |
ppisar
added a commit
to ppisar/librepo
that referenced
this issue
Feb 12, 2024
If librepo was built with -DWITH_ZCHUNK=ON but an application included <librepo/librepo.h> without defining -DWITH_ZCHUNK, struct LrDownloadTarget mismatched. The same problem happened vice versa. Following the correct ABI was a burden for librepo applications and a mistake there lead to a memory corruption. This patch replaces the WITH_ZCHUNK macros in the public header files with a constant so that the installed header files do not depend on the macro and always match the installed library. This patch also adds a missing RPM dependency on zchunk-devel to librepo-devel if built with enabled zchunk. (Some header files include <zck.h>.) An implementation note: CMakeLists.txt appends the header search paths with both a top-level build direcory as well as with a librepo subdirectory. The latter is for "util.h" inclusions from the C files, the former is for <librepo/util.h> inclusions from other librepo header files. Without the former a C preprocesor would use already installed <librepo/util.h>. We do not want to infect the being built library with old and possible nonmatching system header files. Fixes: rpm-software-management#298
ppisar
added a commit
to ppisar/librepo
that referenced
this issue
Feb 12, 2024
If librepo was built with -DWITH_ZCHUNK=ON but an application included <librepo/librepo.h> without defining -DWITH_ZCHUNK, struct LrDownloadTarget mismatched. The same problem happened vice versa. Following the correct ABI was a burden for librepo applications and a mistake there lead to a memory corruption. This patch replaces the WITH_ZCHUNK macros in the public header files with a constant so that the installed header files do not depend on the macro and always match the installed library. This patch also adds a missing RPM dependency on zchunk-devel to librepo-devel if built with enabled zchunk. (Some header files include <zck.h>.) An implementation note: CMakeLists.txt appends the header search paths with both a top-level build direcory as well as with a librepo subdirectory. The latter is for "util.h" inclusions from the C files, the former is for <librepo/util.h> inclusions from other librepo header files. Without the former a C preprocesor would use already installed <librepo/util.h>. We do not want to infect the being built library with old and possible nonmatching system header files. Fixes: rpm-software-management#298
ppisar
added a commit
to ppisar/librepo
that referenced
this issue
Feb 12, 2024
If librepo was built with -DWITH_ZCHUNK=ON but an application included <librepo/librepo.h> without defining -DWITH_ZCHUNK, struct LrDownloadTarget mismatched. The same problem happened vice versa. Following the correct ABI was a burden for librepo applications and a mistake there could lead to a memory corruption. This patch replaces the WITH_ZCHUNK macros in the public header files with a constant so that the installed header files do not depend on the macro and always match the installed library. This patch also adds a missing RPM dependency on zchunk-devel to librepo-devel if built with enabled zchunk. (Some header files include <zck.h>.) An implementation note: CMakeLists.txt appends the header search paths with both a top-level build direcory as well as with a librepo subdirectory. The latter is for "util.h" inclusions from the C files, the former is for <librepo/util.h> inclusions from other librepo header files. Without the former a C preprocesor would use already installed <librepo/util.h>. We do not want to infect the being built library with old and possible nonmatching system header files. Fixes: rpm-software-management#298
ppisar
added a commit
to ppisar/librepo
that referenced
this issue
Feb 12, 2024
If librepo was built with -DWITH_ZCHUNK=ON but an application included <librepo/librepo.h> without defining -DWITH_ZCHUNK, struct LrDownloadTarget mismatched. The same problem happened vice versa. Following the correct ABI was a burden for librepo applications and a mistake there could lead to a memory corruption. This patch replaces the WITH_ZCHUNK macros in the public header files with a constant so that the installed header files do not depend on the macro and always match the installed library. This patch also adds a missing RPM dependency on zchunk-devel to librepo-devel if built with enabled zchunk. (Some header files include <zck.h>.) An implementation note: CMakeLists.txt appends the header search paths with both a top-level build directory as well as with a librepo subdirectory. The latter is for "util.h" inclusions from the C files, the former is for <librepo/util.h> inclusions from other librepo header files. Without the former a C preprocesor would use already installed <librepo/util.h>. We do not want to infect the being built library with old and possible nonmatching system header files. Fixes: rpm-software-management#298
kontura
pushed a commit
that referenced
this issue
Mar 7, 2024
If librepo was built with -DWITH_ZCHUNK=ON but an application included <librepo/librepo.h> without defining -DWITH_ZCHUNK, struct LrDownloadTarget mismatched. The same problem happened vice versa. Following the correct ABI was a burden for librepo applications and a mistake there could lead to a memory corruption. This patch replaces the WITH_ZCHUNK macros in the public header files with a constant so that the installed header files do not depend on the macro and always match the installed library. This patch also adds a missing RPM dependency on zchunk-devel to librepo-devel if built with enabled zchunk. (Some header files include <zck.h>.) An implementation note: CMakeLists.txt appends the header search paths with both a top-level build directory as well as with a librepo subdirectory. The latter is for "util.h" inclusions from the C files, the former is for <librepo/util.h> inclusions from other librepo header files. Without the former a C preprocesor would use already installed <librepo/util.h>. We do not want to infect the being built library with old and possible nonmatching system header files. Fixes: #298
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
DNF5 was asked to drop a useless dependency on zchunk rpm-software-management/dnf5#1233 because it delegates all zchunk work to librepo. However it turned out that
struct LrDownloadTargetABI defined in<librepo/downloadtarget.h>changes withWITH_ZCHUNKmacro.More over, if the macro is defined,
<zck.h>is included albeitlibrepo-develRPM package does not depend onzchunk-devel.All that pushes librepo internals to it users, e.g. to DNF5, and makes the build chain more error prone because the whole aplication stack needs to keep WITH_ZCHUNK in synchronization.
I would appreciate isolating librepo ABI from WITH_ZCHUNK macro.
The text was updated successfully, but these errors were encountered: