PengZheng commented on code in PR #470:
URL: https://github.com/apache/celix/pull/470#discussion_r1382496338
##########
libs/utils/include/celix_version.h:
##########
@@ -20,6 +20,11 @@
#ifndef CELIX_CELIX_VERSION_H
#define CELIX_CELIX_VERSION_H
+#include <stdbool.h>
+#include <stdlib.h>
+
+#include "celix_utils_export.h"
Review Comment:
The above includes duplicate the following includes.
##########
libs/utils/src/version.c:
##########
@@ -95,15 +97,20 @@ celix_status_t version_isCompatible(version_pt user,
version_pt provider, bool*
return CELIX_SUCCESS;
}
-celix_version_t* celix_version_createVersion(int major, int minor, int micro,
const char* qualifier) {
+celix_version_t* celix_version_createVersion(int major, int minor, int micro,
const char * qualifier) {
+ return celix_version_create(major, minor, micro, qualifier);
+}
+
Review Comment:
```suggestion
```
Not used any more.
##########
libs/utils/include/celix_version.h:
##########
@@ -77,27 +97,50 @@ CELIX_UTILS_EXPORT celix_version_t*
celix_version_copy(const celix_version_t* ve
*
* There must be no whitespace in version.
*
- * @param versionStr String representation of the version identifier.
+ * @param[in] versionStr String representation of the version identifier.
* @return The created version or NULL if the input was invalid.
*/
CELIX_UTILS_EXPORT celix_version_t*
celix_version_createVersionFromString(const char *versionStr);
/**
- * The empty version "0.0.0".
- *
+ * @brief Create empty version "0.0.0".
*/
CELIX_UTILS_EXPORT celix_version_t* celix_version_createEmptyVersion();
+/**
+ * @brief Gets the major version number of a celix version.
+ *
+ * @param[in] version The celix version.
+ * @return The major version number.
+ */
CELIX_UTILS_EXPORT int celix_version_getMajor(const celix_version_t* version);
+/**
+ * @brief Gets the minor version number of a celix version.
+ *
+ * @param[in] version The celix version.
+ * @return The minor version number.
+ */
CELIX_UTILS_EXPORT int celix_version_getMinor(const celix_version_t* version);
+/**
+ * @brief Gets the micro version number of a celix version.
+ *
+ * @param[in] version The celix version.
+ * @return The micro version number.
+ */
CELIX_UTILS_EXPORT int celix_version_getMicro(const celix_version_t* version);
+/**
+ * @brief Gets the version qualifier of a celix version.
+ *
+ * @param[in] version The celix version.
+ * @return The version qualifier, or NULL if no qualifier is present.
Review Comment:
Empty string if no qualifier is present?
##########
libs/utils/src/celix_hash_map.c:
##########
@@ -73,10 +61,11 @@ typedef struct celix_hash_map {
bool (*equalsKeyFunction)(const celix_hash_map_key_t* key1, const
celix_hash_map_key_t* key2);
void (*simpleRemovedCallback)(void* value);
void* removedCallbackData;
- void (*removedStringKeyCallback)(void* data, const char* removedKey,
celix_hash_map_value_t removedValue);
- void (*removedLongKeyCallback)(void* data, long removedKey,
celix_hash_map_value_t removedValue);
+ void (*removedStringEntryCallback)(void* data, const char* removedKey,
celix_hash_map_value_t removedValue);
Review Comment:
Is `emptyValue` unused? If so, it should be removed.
##########
libs/utils/src/version.c:
##########
@@ -289,6 +301,15 @@ char* celix_version_toString(const celix_version_t*
version) {
return string;
}
+bool celix_version_fillString(const celix_version_t* version, char *str,
size_t strLen) {
Review Comment:
The return value of `asprintf` in `celix_version_toString` is not checked.
##########
libs/utils/include/celix_version.h:
##########
@@ -30,37 +35,52 @@ extern "C" {
#include "celix_utils_export.h"
/**
- * The definition of the celix_version_t* abstract data type.
+ * @file celix_version.h
+ * @brief Header file for the Celix Version API.
+ *
+ * The Celix Version API provides a means for storing and manipulating version
information, which consists of
+ * three non-negative integers for the major, minor, and micro version, and an
optional string for the qualifier.
+ * This implementation is based on the Semantic Versioning specification
(SemVer).
+ * Functions are provided for creating and destroying version objects,
comparing versions, and extracting the individual version components.
+ */
+
+/**
+ * @brief The definition of the celix_version_t* abstract data type.
*/
typedef struct celix_version celix_version_t;
/**
- * Creates a new celix_version_t* using the supplied arguments.
+ * @brief Create a new celix_version_t* using the supplied arguments.
*
- * @param major Major component of the version identifier.
- * @param minor Minor component of the version identifier.
- * @param micro Micro component of the version identifier.
- * @param qualifier Qualifier component of the version identifier. If
- * <code>null</code> is specified, then the qualifier will be set to
+ * @param[in] major Major component of the version identifier.
+ * @param[in] minor Minor component of the version identifier.
+ * @param[in] micro Micro component of the version identifier.
+ * @param[in] qualifier Qualifier component of the version identifier. If
+ * <code>NULL</code> is specified, then the qualifier will be set to
* the empty string.
* @return The created version or NULL if the input was incorrect
*/
-CELIX_UTILS_EXPORT celix_version_t* celix_version_createVersion(int major, int
minor, int micro, const char* qualifier);
+CELIX_UTILS_EXPORT celix_version_t* celix_version_create(int major, int minor,
int micro, const char* qualifier);
+
+/**
+ * @deprecated Use celix_version_create instead.
+ */
+CELIX_UTILS_DEPRECATED_EXPORT celix_version_t* celixversion_createVersion(int
major, int minor, int micro, const char* qualifier);
Review Comment:
```suggestion
```
Not used anymore.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
