This is an automatically generated e-mail. To reply, visit: http://reviewboard-openlmi.rhcloud.com/r/1657/

On April 30th, 2014, 1:32 p.m. CEST, Radek Novacek wrote:

src/libs/libopenlmi/openlmi.h (Diff revision 1)
35
  #define BUFLEN   1024
Is it really good idea to have all the buffer with same length through all providers?

1024 bytes might be quite big for many usages. The provider itself (= its author) should know how big buffer it uses.
I don't see much difference between buffers of 128/256/1000 bytes or so and 1024. None of the providers does anything that would require such "optimization". If any provider ever needs buffers that are different in size, it can provide its own definition (based on BUFLEN, for example). I really doubt that it's needed though.

Big plus is that there are no more multiple buffer-size-like definitions throughout the sources.

On April 30th, 2014, 1:32 p.m. CEST, Radek Novacek wrote:

src/libs/libopenlmi/openlmi.h (Diff revision 1)
void lmi_set_log_level(int level);
199
#define debug lmi_debug
200
#define warn  lmi_warn
201
#define error lmi_error
I wouldn't do this. I could cause collisions - debug, warn and error are just too common words.
I wouldn't do this too, it is what I have gathered from the original 'globals.h'. It's been there since forever;) I'll remove those.

On April 30th, 2014, 1:32 p.m. CEST, Radek Novacek wrote:

src/libs/libopenlmi/CMakeLists.txt (Diff revision 1)
5
set(OPENLMI_VERSION_CURRENT 1)
6
set(OPENLMI_VERSION_REVISION 0)
7
set(OPENLMI_VERSION_AGE 2)
8
set(OPENLMI_VERSION "${OPENLMI_VERSION_CURRENT}.${OPENLMI_VERSION_REVISION}.${OPENLMI_VERSION_AGE}")
Wouldn't it be better to keep the library versioning consistent with the providers (mof class versioning):

MAJOR.MINOR.PATCH, where
MAJOR is backwards incompatible version,
MINOR is backwards compatible with new features
PATCH is compatible with bugfixes.

Has libtool any advantage to this approach?
It may be better to keep it consistent with the MOF files, yes. However, it think, since it is a library, we should use something more common for a library, which libtool versioning is.
I'm not sure if it is any better than the libtool's version, since it hasn't really been time-tested yet.

Here's the algorithm for using the libtool versioning. Note that you have to go through all of the steps for it to work. You can judge for yourself which is better.

    Start with version information of ‘0:0:0’ for each libtool library. (MY NOTE: we started at 0.0.1, but I don't think that matters)
    Update the version information only immediately before a public release of your software. More frequent updates are unnecessary, and only guarantee that the current interface number gets larger faster.
    If the library source code has changed at all since the last update, then increment revision (‘c:r:a’ becomes ‘c:r+1:a’).
    If any interfaces have been added, removed, or changed since the last update, increment current, and set revision to 0.
    If any interfaces have been added since the last public release, then increment age.
    If any interfaces have been removed or changed since the last public release, then set age to 0. 

- Jan


On April 30th, 2014, 1:04 p.m. CEST, Jan Synacek wrote:

Review request for OpenLMI Developers.
By Jan Synacek.

Updated April 30, 2014, 1:04 p.m.

Repository: openlmi-providers

Description

Make naming consistent. Gather common functionality into one library and
try to use it across all providers.

Introduce libtool-style versioning for libraries.

Note: Sorry for not splitting this humongous patch into parts, I just couldn't find a way that would logically fit.

Diffs

  • openlmi-providers.spec (ed0c91f53c3122f424ac7ccef0c73b5de33f792d)
  • src/CMakeLists.txt (888b59a386f34fb8f3f981aafffb1a5589858ccb)
  • src/account/CMakeLists.txt (83ca474a00a3a8a514bed4fc787360563ad56d01)
  • src/account/LMI_AccountCapabilitiesProvider.c (d304257610c18ebb18ac07925761dcbc224f9f6c)
  • src/account/LMI_AccountManagementCapabilitiesProvider.c (3909603d92205708c191891aaf7c0e9bb3c719f5)
  • src/account/LMI_AccountManagementServiceCapabilitiesProvider.c (efece85f5bf1f8a545220194c7e5b4cc4c63006a)
  • src/account/LMI_AccountManagementServiceProvider.c (21473eaeececde7a6a35f34019aa669712c27f18)
  • src/account/LMI_AccountOnSystemProvider.c (8bcd8351bb10c768bb4bccc059784ef1057861e1)
  • src/account/LMI_AccountProvider.c (ca9758a6d112d37929d0affbb90183565189fd4b)
  • src/account/LMI_AccountSettingDataProvider.c (e132a03b01512cece6b30398c88ca7b0b40c06e2)
  • src/account/LMI_AssignedAccountIdentityProvider.c (181045558220af629c52d29086596a12a9d75793)
  • src/account/LMI_AssignedGroupIdentityProvider.c (2950dc0e2f2a0a400162b0debd7605e17516106a)
  • src/account/LMI_EnabledAccountCapabilitiesProvider.c (319e92685f8331dfde70d061dba205f1a221bc9d)
  • src/account/LMI_GroupProvider.c (20d93b5e79ad736cd96e67506775efb000380556)
  • src/account/LMI_HostedAccountManagementServiceProvider.c (5d7b39168f8c8af3e057c3c5368d6696886bf50e)
  • src/account/LMI_IdentityProvider.c (b86bff014f12e0a93adb5536e8f786dee5df6dc6)
  • src/account/LMI_MemberOfGroupProvider.c (8751597902df11c50a0e8723564c6894094d2593)
  • src/account/LMI_OwningGroupProvider.c (2fef2c889e06b0d360fa4b652ddd553dfe1cb6cc)
  • src/account/LMI_ServiceAffectsIdentityProvider.c (5eccab4d6878b09d94c8a5ba4937cb02a206e263)
  • src/account/indication_common.h (14e41ffef30758a4cd5f6b15ec57d207efa8db27)
  • src/account/indication_common.c (d9629b44c1bce92f8fe0aee25b7f7e48560ec0e1)
  • src/account/macros.h (bd11e0b6b9ffb19e63e61d8f6d1dee9760b60259)
  • src/common/CMakeLists.txt (129719f2ff3be814202f210ca66c9bb1eb7b95b1)
  • src/common/openlmi.c (508ffbaa1f7aeb8dfe3bdf8fa55e7acbd167d9cd)
  • src/common/openlmi.conf ()
  • src/common/openlmi.h (a46544c82894b40d0344b80c5c9b21008a3c701e)
  • src/common/openlmi.pc.in (939d6723f7be27af150b1306f3648f1c72134b70)
  • src/fan/CMakeLists.txt (446e38b820ad53ae28b619df538c49f03a33bb69)
  • src/fan/LMI_FanAssociatedSensorProvider.c (249fdadefd7536588ecaec5f3e1506c1f88ac3d2)
  • src/fan/LMI_FanProvider.c (9e780ad20f2f8449decb5ddbb3374daa2eec2f92)
  • src/fan/LMI_FanSensorProvider.c (70e1be2325780cf0d925f6a0eb5ffee9da076f9b)
  • src/fan/fan.h (6115ab203851ae6e4e0f4f477eaa4bbb481924d8)
  • src/fan/fan.c (adc49cce3c205ef7e24e423884558a25231ee0aa)
  • src/globals.h (128ba0aa51b2a791d979b4269e7fb7bee8f675bb)
  • src/hardware/CMakeLists.txt (29b63d342039a2e19a1fcdfe626cfab2b3a3002f)
  • src/hardware/LMI_AssociatedProcessorCacheMemoryProvider.c (d786f3ddbc023633c1df29c39cdcdcb9bc1ca8da)
  • src/hardware/LMI_BaseboardContainerProvider.c (4fb2d371acfaf16cc37b15119b8c1bcacf6d7f53)
  • src/hardware/LMI_BaseboardProvider.c (fd7d65b835a994d570466d780596105347c327b3)
  • src/hardware/LMI_BatteryPhysicalPackageProvider.c (3b17aa2d20e6d8c7a8205e043059ff665c6be8a4)
  • src/hardware/LMI_BatteryProvider.c (133ecd8efbb13eb04f2ca3706bd72b45862e9422)
  • src/hardware/LMI_BatterySystemDeviceProvider.c (ce2acf02a9af532aa2456d02e8720ac399a1d9f8)
  • src/hardware/LMI_ChassisComputerSystemPackageProvider.c (e63faaa7563fd49a9e19432566e6e5712eda21e2)
  • src/hardware/LMI_ChassisProvider.c (ef8567ad3af0f7752944088d39c8d12a51daaf94)
  • src/hardware/LMI_DiskDriveATAPortProvider.c (122b3aac8d3c09eb1c152fdb3c8d0ae8f9b5935f)
  • src/hardware/LMI_DiskDriveATAProtocolEndpointProvider.c (0e13d9e5e90ee38ee9d226d7b0ea8395c28c0c93)
  • src/hardware/LMI_DiskDriveDeviceSAPImplementationProvider.c (f34ae8f63618f211a5fc6869fe9f5957474ee2f7)
  • src/hardware/LMI_DiskDriveElementSoftwareIdentityProvider.c (ba7937113b34d6b1a17f266e9ce3dac7bf352bf8)
  • src/hardware/LMI_DiskDriveProvider.c (feaae00673dd932e4edc13529788b3dd9a3ce816)
  • src/hardware/LMI_DiskDriveRealizesProvider.c (0c40fdbc073da94851ad9037bce1ac931385233a)
  • src/hardware/LMI_DiskDriveSAPAvailableForElementProvider.c (ee964b1ade1a8a4037caac05139549d09e45107c)
  • src/hardware/LMI_DiskDriveSoftwareIdentityProvider.c (528ad519561674b268ffe6583089d5093423ffc2)
  • src/hardware/LMI_DiskDriveSystemDeviceProvider.c (e29997ae736d6545eb3577b34194a5bd589a4c57)
  • src/hardware/LMI_DiskPhysicalPackageContainerProvider.c (b9c40f570ae86e7082156a9a432b223c958cba65)
  • src/hardware/LMI_DiskPhysicalPackageProvider.c (001205baab621c55a89d4f7eeb1f7c3730dbcffa)
  • src/hardware/LMI_Hardware.h (29d8c9be41371f1790a24112a60fce7623d217ae)
  • src/hardware/LMI_MemoryPhysicalPackageInConnectorProvider.c (51a619288dfbbd9cffdf3f9c612a30a144bd980e)
  • src/hardware/LMI_MemoryPhysicalPackageProvider.c (fc82af1ad514412e6b59b069a2d61f3dd5aa895b)
  • src/hardware/LMI_MemoryProvider.c (e0dcfef97d587d48ffee0243d4203440cd7928d0)
  • src/hardware/LMI_MemorySlotContainerProvider.c (6dd21d3ca9906d40516ffbf5f1cbbb287868d79d)
  • src/hardware/LMI_MemorySlotProvider.c (d3c4a826a50d7acf17a78a46eecbcdc6b7f344b8)
  • src/hardware/LMI_MemorySystemDeviceProvider.c (1eef31fe49d79ebd32984e203114d54b9fdb0f9a)
  • src/hardware/LMI_PCIBridgeProvider.c (e84bea3e91a021777e663fe22b6793a35e4f1c66)
  • src/hardware/LMI_PCIBridgeSystemDeviceProvider.c (1b26613132d2e23ad8657098d25881e940c94354)
  • src/hardware/LMI_PCIDeviceProvider.c (1c94915b0adee5f97ca150badc532bfc881685b4)
  • src/hardware/LMI_PCIDeviceSystemDeviceProvider.c (cb011e9c9447c998ea3a18a1be52466d13833aa0)
  • src/hardware/LMI_PhysicalBatteryContainerProvider.c (4972641f6298522a733fb45d268fbf70047ee402)
  • src/hardware/LMI_PhysicalBatteryRealizesProvider.c (e35b4c6b14e76d05edd6aa31e78ce1886bbcc47a)
  • src/hardware/LMI_PhysicalMemoryContainerProvider.c (3c995e43322f4e752547ad59600e6a4cd6dda408)
  • src/hardware/LMI_PhysicalMemoryProvider.c (c859671504340b1a0d9e4e9e684d2f4c991dc3de)
  • src/hardware/LMI_PhysicalMemoryRealizesProvider.c (d7ec945c43e680059d535a817fb80374871eae4e)
  • src/hardware/LMI_PointingDeviceProvider.c (d7966e909d97490f57103ea302f2c9ec242c65b6)
  • src/hardware/LMI_PortPhysicalConnectorContainerProvider.c (c6a49d882b13dd11ee246d2b05695384beba6bf6)
  • src/hardware/LMI_PortPhysicalConnectorProvider.c (546796091eaa099b06b34a4e64ff096a2a92f591)
  • src/hardware/LMI_ProcessorCacheMemoryProvider.c (b8e0bfe7b9ed929dce0910f25360d4da49fec017)
  • src/hardware/LMI_ProcessorCapabilitiesProvider.c (906b7c2cecebfc838c457b3925e0ca9b44866587)
  • src/hardware/LMI_ProcessorChipContainerProvider.c (5c524eb1cb00a58c36b9ce0d6e4c8b3f823a8428)
  • src/hardware/LMI_ProcessorChipProvider.c (946758ff35346975d1c4d518e921db382c7d66b1)
  • src/hardware/LMI_ProcessorChipRealizesProvider.c (6a126cc48be19890ea4eaccb18d1026d0b6b9fcd)
  • src/hardware/LMI_ProcessorElementCapabilitiesProvider.c (53720003ea06fcb7961355d001aae28e91612c93)
  • src/hardware/LMI_ProcessorProvider.c (a7d2c2c02534d7fafed2f7e752bec7e0f80d8492)
  • src/hardware/LMI_ProcessorSystemDeviceProvider.c (0335e18f5e01d6b72d8c3f20ad79faa9f810e609)
  • src/hardware/LMI_SystemSlotContainerProvider.c (ea4c95e73788e94243c3639638755fef0cdd2dcb)
  • src/hardware/LMI_SystemSlotProvider.c (0cb73cb222a1b2c0acce7d6ae91c862f7d7ed006)
  • src/hardware/lsblk.c (512ccad300e4bb27fa9548f5acf94e6ecafa9b8b)
  • src/hardware/smartctl.c (d637acc73044ac5cf2ec55e3e456e23d7af5625b)
  • src/hardware/sysfs.c (2e02081f8f30911fd6abc0d32dc629ebf8650391)
  • src/hardware/utils.h (17f6d4d7ef3562e24c41e224e43ce3a4f8897f99)
  • src/hardware/utils.c (4193fd91527bf7d7b615a2ebc82c2d1455e0cd7c)
  • src/journald/CMakeLists.txt (1e6f19372172e5410f7b902ab5f61cac262fdf69)
  • src/journald/LMI_JournalLogRecordInstanceCreationIndicationProvider.c (7eb57f7f623884119f05975f613ce8064077dc8f)
  • src/journald/LMI_JournalLogRecordProvider.c (cf49535460721f6c72c6eee3923771a73f93e958)
  • src/journald/LMI_JournalMessageLogProvider.c (297a7755a1c3b4f236ee9a1731bcaee30279dab3)
  • src/journald/LMI_JournalRecordInLogProvider.c (84f3960635ba5a0cc046d76c4631bab532b10dd4)
  • src/journald/instutil.c (413a8b1d43222935d358ee0d71a77b7d642d1a82)
  • src/libs/libopenlmi/CMakeLists.txt (PRE-CREATION)
  • src/logicalfile/CMakeLists.txt (3a09a4ad7a545f8a74f73175461943516e3f8fde)
  • src/logicalfile/LMI_DataFileProvider.c (6abf9a4e1c24c6cdc6376696ade69ca29d9da049)
  • src/logicalfile/LMI_DirectoryContainsFileProvider.c (2033804183bc954e42c7ccf79eaffbb4c509147f)
  • src/logicalfile/LMI_FIFOPipeFileProvider.c (49062bd21fa0ab4e37368433152cfcdba530270d)
  • src/logicalfile/LMI_FileIdentityProvider.c (82010b890fdb6cad1eb96d2f2f4d3307bfe69d2f)
  • src/logicalfile/LMI_RootDirectoryProvider.c (9c2a2db93c437ae8bfcdcc48b22ebaa7a746851f)
  • src/logicalfile/LMI_SymbolicLinkProvider.c (e200316447374228393967d4cc6dfb75245940c9)
  • src/logicalfile/LMI_UnixDeviceFileProvider.c (ab1ec01a582cb5c0c6510655fec8b0aaecb8e6fa)
  • src/logicalfile/LMI_UnixDirectoryProvider.c (0f19cbb580327a423fbb6b5075a1fce2f78e830c)
  • src/logicalfile/LMI_UnixFileProvider.c (b17da5f6a23ca94dd3c469477471f5d907d00ec1)
  • src/logicalfile/LMI_UnixSocketProvider.c (a637f336a8757eda502e6fd3bfa5d468fbbb1c25)
  • src/logicalfile/file.h (d2f573b45e348514ae3bb47ddea78b74bd35910d)
  • src/logicalfile/file.c (ac68b6f53d620aad413f4551dfc1faf40419022c)
  • src/power/CMakeLists.txt (44b2109ec5ce0c41c6d7091144bd58c0a86e84a2)
  • src/power/LMI_AssociatedPowerManagementServiceProvider.c (74c426f6611f134ad8bf59a55ab19d2eaeaddd90)
  • src/power/LMI_ElementCapabilitiesProvider.c (25c2865f465e211432f1737fc11a483b8ce54288)
  • src/power/LMI_HostedPowerManagementServiceProvider.c (9663ed17e2cf04767ccab15ef693a28915279414)
  • src/power/LMI_PowerConcreteJobProvider.c (ef6e50191518ef3fb8c996985958449c73d9bde3)
  • src/power/LMI_PowerManagementCapabilitiesProvider.c (ca0208356c52a46b0e0624f30953f9a2433cce2e)
  • src/power/LMI_PowerManagementServiceProvider.c (9759b1c89701ba0f4c5039436d91372b725f64e2)
  • src/power/power.h (48a9409c88ee9af3a6e3116090e9a2d2decb2f49)
  • src/power/power.c (6ec24b61548156a63d688a175d8fc860fc7e848f)
  • src/realmd/CMakeLists.txt (dbf2e5463d9192d6dc1dbf767c7be1242e67eb11)
  • src/realmd/LMI_HostedRealmdServiceProvider.c (37d93fce78933d390d81e9c9fbfe5a92b8a345dd)
  • src/realmd/LMI_RealmdServiceProvider.c (448fe49212d2c4f77069da9b4b784d5b9c9b4a2e)
  • src/realmd/rdcp_error.h (ff60d5bfb3bd97d01e44d4b921350410a2f3070f)
  • src/realmd/rdcp_error.c (93d7ac387134bb47c836f301e291b4ca817f776e)
  • src/realmd/rdcp_util.h (fb7dc85478087abe47b049a05049d794bcddf3f9)
  • src/realmd/rdcp_util.c (30cb790d0f52969abaca849605db5d5c162e1b1f)
  • src/service-dbus/CMakeLists.txt (4ba32316993c78ec6ae41287fc97867e23fcf098)
  • src/service-dbus/LMI_ServiceProvider.c (709faa5e7504ff2c5d2f35debddb7d7012564d95)
  • src/service-dbus/util/serviceutil.h (c5b0abf32d34e851d9604b372acecc2becca7f6d)
  • src/service/CMakeLists.txt (6d75d38247a8c06b8f6b5e5f4d3b1f8615c910d9)
  • src/service/LMI_ServiceProvider.c (034418e4d2c91bc433d7f41ac91c04879e007bbf)
  • src/service/util/serviceutil.h (d0b666364d471622d322ed29b4c22860efe8ea53)
  • src/service/util/serviceutil.c (121360d52b29bd7b1c4e21ec0bdb1782b93b071d)
  • src/software-dbus/CMakeLists.txt (5202c33e302bca4b1a48f71bcdf884ff4e1ba01e)

View Diff