Merge into trunk : lp-1552082 : Code : Unity API

Reviewer	Review Type	Date Requested	Status
Unity8 CI Bot	continuous-integration		Approve on 2016-03-30
Michi Henning (community)			Approve on 2016-03-30
Paweł Stołowski (community)		2016-03-14	Needs Fixing on 2016-03-16
Review via email: mp+288895@code.launchpad.net

Revision history for this message

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-03-14:

#

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity-api-ci/34/rebuild

review: Approve (continuous-integration)

Revision history for this message

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-03-14:

#

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity-api-ci/35/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-03-14:

#

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity-api-ci/36/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Paweł Stołowski (stolowski) wrote on 2016-03-14:

#

Looks good! Only problem is missing file locking, mention in the review of https://code.launchpad.net/~marcustomlinson/unity-scopes-shell/lp-1552082/+merge/288918

Also, can you mention in the headers docs that sync() may throw? Can you add a test case for exception on sync()?

review: Needs Fixing

Revision history for this message

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-03-15:

#

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity-api-ci/37/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-03-15:

#

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity-api-ci/38/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Marcus Tomlinson (marcustomlinson) wrote on 2016-03-15:

#

> Looks good! Only problem is missing file locking, mention in the review of
> https://code.launchpad.net/~marcustomlinson/unity-scopes-
> shell/lp-1552082/+merge/288918
>
> Also, can you mention in the headers docs that sync() may throw? Can you add a
> test case for exception on sync()?

done.

Revision history for this message

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-03-15:

#

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity-api-ci/39/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-03-15:

#

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity-api-ci/40/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-03-15:

#

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity-api-ci/41/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Marcus Tomlinson (marcustomlinson) wrote on 2016-03-15:

#

@Pawel, Alright, this is ready for review again.

Revision history for this message

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-03-15:

#

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity-api-ci/42/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Paweł Stołowski (stolowski) wrote on 2016-03-16:

#

Thanks Marcus. These changes look good when considered in isolation, but I'm afaird they have big impact in SettingsDB implementation in the scope. By having file locking implemented on the IniParser level, we're now locking twice (SettingsDB locks the file too); moreover there is some logic around in SettingsDB to give defaults if locking fails etc. If we want to go that route, then I suspect SettingsDB will need changes... Should we consider manual locking (by the called) instead? Let's discuss that in the standup.

review: Needs Fixing

Revision history for this message

Paweł Stołowski (stolowski) wrote on 2016-03-16:

#

> Thanks Marcus. These changes look good when considered in isolation, but I'm
> afaird they have big impact in SettingsDB implementation in the scope. By
> having file locking implemented on the IniParser level, we're now locking
> twice (SettingsDB locks the file too); moreover there is some logic around in
> SettingsDB to give defaults if locking fails etc. If we want to go that route,
> then I suspect SettingsDB will need changes... Should we consider manual
> locking (by the called) instead? Let's discuss that in the standup.

Uh, sorry, didn't mean to set it to 'needs fixing', but 'needs discussion' instead ;) As I said, nothing wrong with these changes in general, we just need to consider the implications.

review: Needs Information

Revision history for this message

Paweł Stołowski (stolowski) wrote on 2016-03-16:

#

Oh, actually there is a problem.

+ fl.l_type = F_RDLCK;

We're using a read-lock for both reading and writing.

review: Needs Fixing

Revision history for this message

Michi Henning (michihenning) wrote on 2016-03-16:

#

The locking here is at the wrong level of abstraction, I believe. It is not necessary to lock at this level because there will be many cases where the writer and reader are the same single process.

The locking needs to be added at the level above, which knows whether a particular file might be accessed concurrently.

We already have locking in SettingsDB in scopes API. The locking needs to be moved in to the shell, to do the equivalent of what QSettings does.

review: Needs Fixing

Revision history for this message

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-03-17:

#

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity-api-ci/44/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Marcus Tomlinson (marcustomlinson) wrote on 2016-03-17:

#

Indeed, for us to maintain behavioural compatibility, the file locking should occur on in unity-scopes-shell, as it was before via QSettings.

Revision history for this message

Michi Henning (michihenning) wrote on 2016-03-17:

#

One option to deal with this *might* be to make the SettingsDB class visible as part of the public API. That would allow the shell to just use it. Alternatively, steal the code from SettingsDB.cpp and put that into the shell. (It's not a lot of code.)

Revision history for this message

Marcus Tomlinson (marcustomlinson) wrote on 2016-03-17:

#

> Thanks Marcus. These changes look good when considered in isolation, but I'm
> afaird they have big impact in SettingsDB implementation in the scope. By
> having file locking implemented on the IniParser level, we're now locking
> twice (SettingsDB locks the file too); moreover there is some logic around in
> SettingsDB to give defaults if locking fails etc. If we want to go that route,
> then I suspect SettingsDB will need changes... Should we consider manual
> locking (by the called) instead? Let's discuss that in the standup.

Fixed in https://code.launchpad.net/~marcustomlinson/unity-scopes-shell/lp-1552082/+merge/288918

Revision history for this message

Marcus Tomlinson (marcustomlinson) wrote on 2016-03-18:

#

Ok, Michi made a good point this morning that the write functionality added to this class is looking too much like an after thought. As this lib is meant to serve a very wide audience, we need to ensure that the API is complete such that it is useful for wide range of applications (not just the shell plugin).

From a quick look over the API, one example of missing functionality is some way to delete groups and keys.

Let's hold back on landing this MP until I've managed to wrap up the API a little tighter.

Revision history for this message

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-03-24:

#

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity-api-ci/50/rebuild

review: Approve (continuous-integration)

Revision history for this message

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-03-29:

#

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity-api-ci/55/rebuild

review: Approve (continuous-integration)

Revision history for this message

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-03-29:

#

FAILED: Continuous integration, rev:236
https://unity8-jenkins.ubuntu.com/job/lp-unity-api-ci/56/
Executed test runs:
FAILURE: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/1171/console

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity-api-ci/56/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-03-29:

#

FAILED: Continuous integration, rev:237
https://unity8-jenkins.ubuntu.com/job/lp-unity-api-ci/58/
Executed test runs:
FAILURE: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/1176/console

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity-api-ci/58/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-03-29:

#

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity-api-ci/59/rebuild

review: Approve (continuous-integration)

Revision history for this message

Michi Henning (michihenning) wrote on 2016-03-30:

#

Looks good! Nice coverage too! :-)

Some very minor code and doc quibbles below...

In the constructor and destructor, there is no need to lock the mutex.

"To extract / insert a value, simply specify the group and key
names to the get* / set* methods of this class respectively."

The "/" needs to be close set. I'd also remove the "*", and a comma needs to be added:

"To extract/insert a value, simply specify the group and key
names to the get/set methods of this class, respectively."

"The get methods indicate errors by throwing LogicExceptions."

I'd remove the plural:

"The get methods indicate errors by throwing LogicException."

"Examples why this might happen is because a value can't be
coerced into the given type (i.e trying to convert the value
"hello" into a boolean)."

"i.e" is missing a full stop here. But I'd be inclined to delete this sentence entirely because almost the same information is provided in the doc for the read methods below. For completeness, I'd reword that comment as:

/** @name Read Methods
 * These member functions provide read access to configuration entries by group and key. 
 * Attempts to retrieve a value as the wrong type (such as retrieving a string value
 * as an integer) or to retrieve a value for a non-existent group or
 * key throw LogicException.
 **/

In the summary comment for the class, I'd add that all methods are thread-safe.

/** @name Write Methods
 * These member functions provide write access to configuration entries by group and key. 
 * Attempts to remove groups or keys that do not exist, throw LogicException.
 **/

Remove the comma before "throw LogicException." I would also add:

The set methods replace the value for a key if the key exists. Calling a set method for
a non-existent group and/or key creates the group and/or key.

/** @name Sync Method
 * This member function will write unsaved changes back to the configuration file. 
 * A failure to write to file will throw a FileException.
 **/

I would get rid of the future tense:

/** @name Sync Method
 * This member function writes unsaved changes back to the configuration file. 
 * A failure to write to the file throws a FileException.
 **/

Looks good! Nice coverage too! :-)

Some very minor code and doc quibbles below...

In the constructor and destructor, there is no need to lock the mutex.

"To extract / insert a value, simply specify the group and key
 names to the get* / set* methods of this class respectively."

The "/" needs to be close set. I'd also remove the "*", and a comma needs to be added:

"To extract/insert a value, simply specify the group and key
 names to the get/set methods of this class, respectively."

"The get methods indicate errors by throwing LogicExceptions."

I'd remove the plural:

"The get methods indicate errors by throwing LogicException."

"Examples why this might happen is because a value can't be
coerced into the given type (i.e trying to convert the value
"hello" into a boolean)."

"i.e" is missing a full stop here. But I'd be inclined to delete this sentence entirely because almost the same information is provided in the doc for the read methods below. For completeness, I'd reword that comment as:

/** @name Read Methods
 * These member functions provide read access to configuration entries by group and key. 
 * Attempts to retrieve a value as the wrong type (such as retrieving a string value
 * as an integer) or to retrieve a value for a non-existent group or
 * key throw LogicException.
 **/

In the summary comment for the class, I'd add that all methods are thread-safe.

/** @name Write Methods
 * These member functions provide write access to configuration entries by group and key. 
 * Attempts to remove groups or keys that do not exist, throw LogicException.
 **/

Remove the comma before "throw LogicException." I would also add:

The set methods replace the value for a key if the key exists. Calling a set method for
       a non-existent group and/or key creates the group and/or key.

/** @name Sync Method
 * This member function will write unsaved changes back to the configuration file. 
 * A failure to write to file will throw a FileException.
 **/

I would get rid of the future tense:

/** @name Sync Method
 * This member function writes unsaved changes back to the configuration file. 
 * A failure to write to the file throws a FileException.
 **/

review: Needs Fixing

Revision history for this message

Marcus Tomlinson (marcustomlinson) wrote on 2016-03-30:

#

> Looks good! Nice coverage too! :-)
>
> Some very minor code and doc quibbles below...
>
> In the constructor and destructor, there is no need to lock the mutex.

Sorry, I just had to have a look ;P

52.2.27 jussi.p | IniParser::IniParser(const char* filename)
52.2.12 jussi.p | {
120.1.1 michi.h | lock_guard<std::mutex> lock(internal::parser_mutex);

...

52.2.17 jussi.p | IniParser::~IniParser() noexcept
52.2.12 jussi.p | {
120.1.1 michi.h | lock_guard<std::mutex> lock(internal::parser_mutex);

>
> "To extract / insert a value, simply specify the group and key
> names to the get* / set* methods of this class respectively."
>
> The "/" needs to be close set. I'd also remove the "*", and a comma needs to
> be added:
>
> "To extract/insert a value, simply specify the group and key
> names to the get/set methods of this class, respectively."
>
> "The get methods indicate errors by throwing LogicExceptions."
>
> I'd remove the plural:
>
> "The get methods indicate errors by throwing LogicException."
>
> "Examples why this might happen is because a value can't be
> coerced into the given type (i.e trying to convert the value
> "hello" into a boolean)."
>
> "i.e" is missing a full stop here. But I'd be inclined to delete this sentence
> entirely because almost the same information is provided in the doc for the
> read methods below. For completeness, I'd reword that comment as:
>
> /** @name Read Methods
> * These member functions provide read access to configuration entries by
> group and key. 
> * Attempts to retrieve a value as the wrong type (such as retrieving a
> string value
> * as an integer) or to retrieve a value for a non-existent group or
> * key throw LogicException.
> **/
>
> In the summary comment for the class, I'd add that all methods are thread-
> safe.
>
> /** @name Write Methods
> * These member functions provide write access to configuration entries by
> group and key. 
> * Attempts to remove groups or keys that do not exist, throw
> LogicException.
> **/
>
> Remove the comma before "throw LogicException." I would also add:
>
> The set methods replace the value for a key if the key exists. Calling
> a set method for
> a non-existent group and/or key creates the group and/or key.
>
>
> /** @name Sync Method
> * This member function will write unsaved changes back to the
> configuration file. 
> * A failure to write to file will throw a FileException.
> **/
>
> I would get rid of the future tense:
>
> /** @name Sync Method
> * This member function writes unsaved changes back to the configuration
> file. 
> * A failure to write to the file throws a FileException.
> **/

Thanks Michi, done.

> Looks good! Nice coverage too! :-)
> 
> Some very minor code and doc quibbles below...
> 
> In the constructor and destructor, there is no need to lock the mutex.

Sorry, I just had to have a look ;P

52.2.27 jussi.p | IniParser::IniParser(const char* filename)
52.2.12 jussi.p | {
120.1.1 michi.h | lock_guard<std::mutex> lock(internal::parser_mutex);

...

52.2.17 jussi.p | IniParser::~IniParser() noexcept
52.2.12 jussi.p | {
120.1.1 michi.h | lock_guard<std::mutex> lock(internal::parser_mutex);

> 
> "To extract / insert a value, simply specify the group and key
> names to the get* / set* methods of this class respectively."
> 
> The "/" needs to be close set. I'd also remove the "*", and a comma needs to
> be added:
> 
> "To extract/insert a value, simply specify the group and key
> names to the get/set methods of this class, respectively."
> 
> "The get methods indicate errors by throwing LogicExceptions."
> 
> I'd remove the plural:
> 
> "The get methods indicate errors by throwing LogicException."
> 
> "Examples why this might happen is because a value can't be
> coerced into the given type (i.e trying to convert the value
> "hello" into a boolean)."
> 
> "i.e" is missing a full stop here. But I'd be inclined to delete this sentence
> entirely because almost the same information is provided in the doc for the
> read methods below. For completeness, I'd reword that comment as:
> 
> /** @name Read Methods
> * These member functions provide read access to configuration entries by
> group and key. 
> * Attempts to retrieve a value as the wrong type (such as retrieving a
> string value
> * as an integer) or to retrieve a value for a non-existent group or
> * key throw LogicException.
> **/
> 
> In the summary comment for the class, I'd add that all methods are thread-
> safe.
> 
> /** @name Write Methods
> * These member functions provide write access to configuration entries by
> group and key. 
> * Attempts to remove groups or keys that do not exist, throw
> LogicException.
> **/
> 
> Remove the comma before "throw LogicException." I would also add:
> 
> The set methods replace the value for a key if the key exists. Calling
> a set method for
> a non-existent group and/or key creates the group and/or key.
> 
> 
> /** @name Sync Method
> * This member function will write unsaved changes back to the
> configuration file. 
> * A failure to write to file will throw a FileException.
> **/
> 
> I would get rid of the future tense:
> 
> /** @name Sync Method
> * This member function writes unsaved changes back to the configuration
> file. 
> * A failure to write to the file throws a FileException.
> **/

Thanks Michi, done.

Revision history for this message

Michi Henning (michihenning) wrote on 2016-03-30:

#

> Sorry, I just had to have a look ;P

Yes, me bad. That was caused by a false positive from helgrind, from memory. The lock makes the FP go away, because helgrind doesn't understand atomics. I should have gone back and removed this again.

Revision history for this message

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-03-30:

#

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity-api-ci/61/rebuild

review: Approve (continuous-integration)

Revision history for this message

Michi Henning (michihenning) wrote on 2016-03-30:

#

Looks good, thanks!

review: Approve

Revision history for this message

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-03-30:

#

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity-api-ci/62/rebuild

review: Approve (continuous-integration)

Unity API

Merge lp:~marcustomlinson/unity-api/lp-1552082 into lp:unity-api

Commit message

Description of the change

Preview Diff

Subscribers

 === modified file 'CMakeLists.txt'
 --- CMakeLists.txt	2015-08-11 14:05:12 +0000
 +++ CMakeLists.txt	2016-03-30 08:50:57 +0000
@@ -122,7 +122,7 @@
  # API version
  set(UNITY_API_MAJOR 0)
  set(UNITY_API_MINOR 1)
--set(UNITY_API_MICRO 5)
++set(UNITY_API_MICRO 6)
  set(UNITY_API_VERSION "${UNITY_API_MAJOR}.${UNITY_API_MINOR}.${UNITY_API_MICRO}")
  # API library
 === modified file 'debian/changelog'
 --- debian/changelog	2016-03-22 07:48:55 +0000
 +++ debian/changelog	2016-03-30 08:50:57 +0000
@@ -1,3 +1,9 @@
++unity-api (7.109-0ubuntu1) UNRELEASED; urgency=medium
++
++  * Add set*() methods to IniParser
++
++ -- Marcus Tomlinson <marcus.tomlinson@canonical.com>  Tue, 29 Mar 2016 08:32:15 +0200
++
  unity-api (7.108+16.04.20160322-0ubuntu1) xenial; urgency=medium
    [ Lukáš Tinkl, Michał Sawicz ]
 === modified file 'include/unity/util/IniParser.h'
 --- include/unity/util/IniParser.h	2014-07-03 05:00:00 +0000
 +++ include/unity/util/IniParser.h	2016-03-30 08:50:57 +0000
@@ -22,8 +22,8 @@
  #include <unity/SymbolExport.h>
  #include <unity/util/DefinesPtrs.h>
--#include<string>
--#include<vector>
++#include <string>
++#include <vector>
  namespace unity
+ {
@@ -37,11 +37,11 @@
+ }
  /**
--\brief Helper class to read configuration files.
++\brief Helper class to read and write configuration files.
  This class reads configuration files in the .ini format
  and provides for a simple and type safe way of extracting
--information. A typical ini file looks like this:
++and inserting information. A typical ini file looks like this:
  ~~~
  [group1]
@@ -55,14 +55,17 @@
  key2 = othervalue2
  ~~~
--To obtain a value, simply specify the group and key names to
--the get* functions of this class. The array functions use
--a semicolon as a separator.
--
--The get functions indicate errors by throwing LogicExceptions.
--Examples why this might happen is because a value can't be
--coerced into the given type (i.e trying to convert the value
--"hello" into a boolean).
++To extract/insert a value, simply specify the group and key
++names to the get/set methods of this class, respectively.
++The array methods use a semicolon as a separator.
++
++To write unsaved changes back to the configuration file, call
++sync(). The sync() method will throw a FileException if it
++fails to write to file.
++
++The get methods indicate errors by throwing LogicException.
++
++All methods are thread-safe.
  */
  class UNITY_API IniParser final {
@@ -78,31 +81,72 @@
      IniParser() = delete;
      /// @endcond
--    /** @name Accessors
--     * These member functions provide access to configuration entries by group and key.
--     * Attempts to retrieve a value as the wrong type, such as retrieving a string value
--     * "abc" as an integer, throw LogicException.
--      **/
      //{@
++
++    /** @name Read Methods
++     * These member functions provide read access to configuration entries by group and key.<br>
++     * Attempts to retrieve a value as the wrong type (such as retrieving a string value as an
++     * integer) or to retrieve a value for a non-existent group or key throw LogicException.
++     **/
++
      bool has_group(const std::string& group) const noexcept;
      bool has_key(const std::string& group, const std::string& key) const;
++
      std::string get_string(const std::string& group, const std::string& key) const;
      std::string get_locale_string(const std::string& group,
                                    const std::string& key,
                                    const std::string& locale = std::string()) const;
      bool get_boolean(const std::string& group, const std::string& key) const;
      int get_int(const std::string& group, const std::string& key) const;
++    double get_double(const std::string& group, const std::string& key) const;
      std::vector<std::string> get_string_array(const std::string& group, const std::string& key) const;
      std::vector<std::string> get_locale_string_array(const std::string& group,
                                                       const std::string& key,
                                                       const std::string& locale = std::string()) const;
++    std::vector<bool> get_boolean_array(const std::string& group, const std::string& key) const;
      std::vector<int> get_int_array(const std::string& group, const std::string& key) const;
--    std::vector<bool> get_boolean_array(const std::string& group, const std::string& key) const;
++    std::vector<double> get_double_array(const std::string& group, const std::string& key) const;
      std::string get_start_group() const;
      std::vector<std::string> get_groups() const;
      std::vector<std::string> get_keys(const std::string& group) const;
++
++    /** @name Write Methods
++     * These member functions provide write access to configuration entries by group and key.<br>
++     * Attempts to remove groups or keys that do not exist throw LogicException.<br>
++     * The set methods replace the value for a key if the key exists. Calling a set method for a
++     * non-existent group and/or key creates the group and/or key.
++      **/
++
++    bool remove_group(const std::string& group);
++    bool remove_key(const std::string& group, const std::string& key);
++
++    void set_string(const std::string& group, const std::string& key, const std::string& value);
++    void set_locale_string(const std::string& group,
++                           const std::string& key,
++                           const std::string& value,
++                           const std::string& locale = std::string());
++    void set_boolean(const std::string& group, const std::string& key, bool value);
++    void set_int(const std::string& group, const std::string& key, int value);
++    void set_double(const std::string& group, const std::string& key, double value);
++
++    void set_string_array(const std::string& group, const std::string& key, const std::vector<std::string>& value);
++    void set_locale_string_array(const std::string& group,
++                                 const std::string& key,
++                                 const std::vector<std::string>& value,
++                                 const std::string& locale = std::string());
++    void set_boolean_array(const std::string& group, const std::string& key, const std::vector<bool>& value);
++    void set_int_array(const std::string& group, const std::string& key, const std::vector<int>& value);
++    void set_double_array(const std::string& group, const std::string& key, const std::vector<double>& value);
++
++    /** @name Sync Method
++     * This member function writes unsaved changes back to the configuration file.<br>
++     * A failure to write to the file throws a FileException.
++      **/
++
++    void sync();
++
      //@}
  private:
 === modified file 'src/unity/util/IniParser.cpp'
 --- src/unity/util/IniParser.cpp	2014-07-03 05:00:00 +0000
 +++ src/unity/util/IniParser.cpp	2016-03-30 08:50:57 +0000
@@ -16,10 +16,12 @@
   * Authored by: Jussi Pakkanen <jussi.pakkanen@canonical.com>
   */
--#include<unity/UnityExceptions.h>
--#include<unity/util/IniParser.h>
--#include<glib.h>
--#include<mutex>
++#include <unity/UnityExceptions.h>
++#include <unity/util/IniParser.h>
++
++#include <mutex>
++
++#include <glib.h>
  using namespace std;
@@ -29,7 +31,6 @@
  namespace util
+ {
--
  namespace internal
+ {
@@ -37,6 +38,7 @@
+ {
      GKeyFile *k;
      string filename;
++    bool dirty = false;
  };
  static std::mutex parser_mutex;
@@ -68,8 +70,6 @@
  IniParser::IniParser(const char* filename)
+ {
--    lock_guard<std::mutex> lock(internal::parser_mutex);
--
      GKeyFile* kf = g_key_file_new();
      GError* e = nullptr;
      if (!kf)
@@ -95,14 +95,14 @@
  IniParser::~IniParser() noexcept
+ {
--    lock_guard<std::mutex> lock(internal::parser_mutex);
--
      g_key_file_free(p->k);
      delete p;
+ }
  bool IniParser::has_group(const std::string& group) const noexcept
+ {
++    lock_guard<std::mutex> lock(internal::parser_mutex);
++
      gboolean rval;
      rval = g_key_file_has_group(p->k, group.c_str());
      return rval;
@@ -110,6 +110,8 @@
  bool IniParser::has_key(const std::string& group, const std::string& key) const
+ {
++    lock_guard<std::mutex> lock(internal::parser_mutex);
++
      gboolean rval;
      GError* e = nullptr;
      rval = g_key_file_has_key(p->k, group.c_str(), key.c_str(), &e);
@@ -119,6 +121,8 @@
  std::string IniParser::get_string(const std::string& group, const std::string& key) const
+ {
++    lock_guard<std::mutex> lock(internal::parser_mutex);
++
      gchar* value;
      GError* e = nullptr;
      string result;
@@ -131,6 +135,8 @@
  std::string IniParser::get_locale_string(const std::string& group, const std::string& key, const std::string& locale) const
+ {
++    lock_guard<std::mutex> lock(internal::parser_mutex);
++
      gchar* value;
      GError* e = nullptr;
      string result;
@@ -143,6 +149,8 @@
  bool IniParser::get_boolean(const std::string& group, const std::string& key) const
+ {
++    lock_guard<std::mutex> lock(internal::parser_mutex);
++
      bool rval;
      GError* e = nullptr;
      rval = g_key_file_get_boolean(p->k, group.c_str(), key.c_str(), &e);
@@ -152,6 +160,8 @@
  int IniParser::get_int(const std::string& group, const std::string& key) const
+ {
++    lock_guard<std::mutex> lock(internal::parser_mutex);
++
      int rval;
      GError* e = nullptr;
      rval = g_key_file_get_integer(p->k, group.c_str(), key.c_str(), &e);
@@ -159,8 +169,21 @@
      return rval;
+ }
++double IniParser::get_double(const std::string& group, const std::string& key) const
++{
++    lock_guard<std::mutex> lock(internal::parser_mutex);
++
++    double rval;
++    GError* e = nullptr;
++    rval = g_key_file_get_double(p->k, group.c_str(), key.c_str(), &e);
++    inspect_error(e, "Could not get double value", p->filename, group);
++    return rval;
++}
++
  std::vector<std::string> IniParser::get_string_array(const std::string& group, const std::string& key) const
+ {
++    lock_guard<std::mutex> lock(internal::parser_mutex);
++
      vector<string> result;
      GError* e = nullptr;
      gchar** strlist;
@@ -179,6 +202,8 @@
                                                              const std::string& key,
                                                              const std::string& locale) const
+ {
++    lock_guard<std::mutex> lock(internal::parser_mutex);
++
      vector<string> result;
      GError* e = nullptr;
      gchar** strlist;
@@ -194,8 +219,28 @@
      return result;
+ }
++vector<bool> IniParser::get_boolean_array(const std::string& group, const std::string& key) const
++{
++    lock_guard<std::mutex> lock(internal::parser_mutex);
++
++    vector<bool> result;
++    GError* e = nullptr;
++    gboolean* bools;
++    gsize count;
++    bools = g_key_file_get_boolean_list(p->k, group.c_str(), key.c_str(), &count, &e);
++    inspect_error(e, "Could not get boolean array", p->filename, group);
++    for (gsize i = 0; i < count; i++)
++    {
++        result.push_back(bools[i]);
++    }
++    g_free(bools);
++    return result;
++}
++
  vector<int> IniParser::get_int_array(const std::string& group, const std::string& key) const
+ {
++    lock_guard<std::mutex> lock(internal::parser_mutex);
++
      vector<int> result;
      GError* e = nullptr;
      gint* ints;
@@ -210,24 +255,28 @@
      return result;
+ }
--vector<bool> IniParser::get_boolean_array(const std::string& group, const std::string& key) const
++vector<double> IniParser::get_double_array(const std::string& group, const std::string& key) const
+ {
--    vector<bool> result;
++    lock_guard<std::mutex> lock(internal::parser_mutex);
++
++    vector<double> result;
      GError* e = nullptr;
--    gboolean* bools;
++    gdouble* doubles;
      gsize count;
--    bools = g_key_file_get_boolean_list(p->k, group.c_str(), key.c_str(), &count, &e);
--    inspect_error(e, "Could not get boolean array", p->filename, group);
++    doubles = g_key_file_get_double_list(p->k, group.c_str(), key.c_str(), &count, &e);
++    inspect_error(e, "Could not get double array", p->filename, group);
      for (gsize i = 0; i < count; i++)
+     {
--        result.push_back(bools[i]);
++        result.push_back(doubles[i]);
+     }
--    g_free(bools);
++    g_free(doubles);
      return result;
+ }
  string IniParser::get_start_group() const
+ {
++    lock_guard<std::mutex> lock(internal::parser_mutex);
++
      gchar* sg = g_key_file_get_start_group(p->k);
      string result(sg);
      g_free(sg);
@@ -236,6 +285,8 @@
  vector<string> IniParser::get_groups() const
+ {
++    lock_guard<std::mutex> lock(internal::parser_mutex);
++
      vector<string> result;
      gsize count;
      gchar** groups = g_key_file_get_groups(p->k, &count);
@@ -249,6 +300,8 @@
  vector<string> IniParser::get_keys(const std::string& group) const
+ {
++    lock_guard<std::mutex> lock(internal::parser_mutex);
++
      vector<string> result;
      GError* e = nullptr;
      gchar** strlist;
@@ -264,6 +317,184 @@
      return result;
+ }
++bool IniParser::remove_group(const std::string& group)
++{
++    lock_guard<std::mutex> lock(internal::parser_mutex);
++
++    gboolean rval;
++    GError* e = nullptr;
++    rval = g_key_file_remove_group(p->k, group.c_str(), &e);
++    inspect_error(e, "Error removing group", p->filename, group);
++    return rval;
++}
++
++bool IniParser::remove_key(const std::string& group, const std::string& key)
++{
++    lock_guard<std::mutex> lock(internal::parser_mutex);
++
++    gboolean rval;
++    GError* e = nullptr;
++    rval = g_key_file_remove_key(p->k, group.c_str(), key.c_str(), &e);
++    inspect_error(e, "Error removing key", p->filename, group);
++    return rval;
++}
++
++void IniParser::set_string(const std::string& group, const std::string& key, const std::string& value)
++{
++    lock_guard<std::mutex> lock(internal::parser_mutex);
++
++    g_key_file_set_string(p->k, group.c_str(), key.c_str(), value.c_str());
++    p->dirty = true;
++}
++
++void IniParser::set_locale_string(const std::string& group, const std::string& key,
++                                  const std::string& value, const std::string& locale)
++{
++    lock_guard<std::mutex> lock(internal::parser_mutex);
++
++    g_key_file_set_locale_string(p->k, group.c_str(), key.c_str(), locale.c_str(), value.c_str());
++    p->dirty = true;
++}
++
++void IniParser::set_boolean(const std::string& group, const std::string& key, bool value)
++{
++    lock_guard<std::mutex> lock(internal::parser_mutex);
++
++    g_key_file_set_boolean(p->k, group.c_str(), key.c_str(), value);
++    p->dirty = true;
++}
++
++void IniParser::set_int(const std::string& group, const std::string& key, int value)
++{
++    lock_guard<std::mutex> lock(internal::parser_mutex);
++
++    g_key_file_set_integer(p->k, group.c_str(), key.c_str(), value);
++    p->dirty = true;
++}
++
++void IniParser::set_double(const std::string& group, const std::string& key, double value)
++{
++    lock_guard<std::mutex> lock(internal::parser_mutex);
++
++    g_key_file_set_double(p->k, group.c_str(), key.c_str(), value);
++    p->dirty = true;
++}
++
++void IniParser::set_string_array(const std::string& group, const std::string& key,
++                                 const std::vector<std::string>& value)
++{
++    lock_guard<std::mutex> lock(internal::parser_mutex);
++
++    int count = value.size();
++    gchar** strlist = g_new(gchar*, count+1);
++
++    for (int i = 0; i < count; ++i)
++    {
++        strlist[i] = g_strdup(value[i].c_str());
++    }
++    strlist[count] = nullptr;
++
++    g_key_file_set_string_list(p->k, group.c_str(), key.c_str(), strlist, count);
++    p->dirty = true;
++
++    g_strfreev(strlist);
++}
++
++void IniParser::set_locale_string_array(const std::string& group, const std::string& key,
++                                        const std::vector<std::string>& value, const std::string& locale)
++{
++    lock_guard<std::mutex> lock(internal::parser_mutex);
++
++    int count = value.size();
++    gchar** strlist = g_new(gchar*, count+1);
++
++    for (int i = 0; i < count; ++i)
++    {
++        strlist[i] = g_strdup(value[i].c_str());
++    }
++    strlist[count] = nullptr;
++
++    g_key_file_set_locale_string_list(p->k, group.c_str(), key.c_str(), locale.c_str(), strlist, count);
++    p->dirty = true;
++
++    g_strfreev(strlist);
++}
++
++void IniParser::set_boolean_array(const std::string& group, const std::string& key, const std::vector<bool>& value)
++{
++    lock_guard<std::mutex> lock(internal::parser_mutex);
++
++    int count = value.size();
++    gboolean* boollist = g_new(gboolean, count);
++
++    for (int i = 0; i < count; ++i)
++    {
++        boollist[i] = value[i];
++    }
++
++    g_key_file_set_boolean_list(p->k, group.c_str(), key.c_str(), boollist, count);
++    p->dirty = true;
++
++    g_free(boollist);
++}
++
++void IniParser::set_int_array(const std::string& group, const std::string& key, const std::vector<int>& value)
++{
++    lock_guard<std::mutex> lock(internal::parser_mutex);
++
++    int count = value.size();
++    gint* intlist = g_new(gint, count);
++
++    for (int i = 0; i < count; ++i)
++    {
++        intlist[i] = value[i];
++    }
++
++    g_key_file_set_integer_list(p->k, group.c_str(), key.c_str(), intlist, count);
++    p->dirty = true;
++
++    g_free(intlist);
++}
++
++void IniParser::set_double_array(const std::string& group, const std::string& key, const std::vector<double>& value)
++{
++    lock_guard<std::mutex> lock(internal::parser_mutex);
++
++    int count = value.size();
++    gdouble* doublelist = g_new(gdouble, count);
++
++    for (int i = 0; i < count; ++i)
++    {
++        doublelist[i] = value[i];
++    }
++
++    g_key_file_set_double_list(p->k, group.c_str(), key.c_str(), doublelist, count);
++    p->dirty = true;
++
++    g_free(doublelist);
++}
++
++void IniParser::sync()
++{
++    lock_guard<std::mutex> lock(internal::parser_mutex);
++
++    if (p->dirty)
++    {
++        GError* e = nullptr;
++        if (!g_key_file_save_to_file(p->k, p->filename.c_str(), &e))
++        {
++            string message = "Could not write ini file ";
++            message += p->filename;
++            message += ": ";
++            message += e->message;
++            int errnum = e->code;
++            g_error_free(e);
++            throw FileException(message, errnum);
++        }
++
++        p->dirty = false;
++    }
++}
  } // namespace util
 === modified file 'test/data/sample.ini'
 --- test/data/sample.ini	2014-07-03 05:00:00 +0000
 +++ test/data/sample.ini	2016-03-30 08:50:57 +0000
@@ -1,6 +1,7 @@
  [first]
  intvalue = 1
++doublevalue = 2.345
  boolvalue = true
  stringvalue = hello
  locstring = world
@@ -17,3 +18,4 @@
  boolvalue = false
  stringvalue = there
  intarray = 4;5;6;78;8;9;9;345;3
++doublearray = 4.5;6.78;9;10.11
 === modified file 'test/gtest/unity/util/IniParser/CMakeLists.txt'
 --- test/gtest/unity/util/IniParser/CMakeLists.txt	2013-06-06 12:02:05 +0000
 +++ test/gtest/unity/util/IniParser/CMakeLists.txt	2016-03-30 08:50:57 +0000
@@ -1,4 +1,6 @@
  add_executable(IniParser_test IniParser_test.cpp)
  target_link_libraries(IniParser_test ${LIBS} ${TESTLIBS})
++add_definitions(-DTEST_RUNTIME_PATH="${CMAKE_CURRENT_BINARY_DIR}")
++
  add_test(IniParser IniParser_test)
 === modified file 'test/gtest/unity/util/IniParser/IniParser_test.cpp'
 --- test/gtest/unity/util/IniParser/IniParser_test.cpp	2014-07-03 05:00:00 +0000
 +++ test/gtest/unity/util/IniParser/IniParser_test.cpp	2016-03-30 08:50:57 +0000
@@ -26,6 +26,7 @@
  using namespace unity::util;
  #define INI_FILE UNITY_API_TEST_DATADIR "/sample.ini"
++#define INI_TEMP_FILE TEST_RUNTIME_PATH "/temp.ini"
  TEST(IniParser, basic)
+ {
@@ -45,11 +46,23 @@
+ {
      IniParser conf(INI_FILE);
--    ASSERT_TRUE(conf.has_group("first"));
++    ASSERT_FALSE(conf.has_key("first", "missingvalue"));
      ASSERT_FALSE(conf.has_group("nonexisting"));
      ASSERT_TRUE(conf.has_key("first", "stringvalue"));
--    ASSERT_FALSE(conf.has_key("first", "missingvalue"));
++    ASSERT_TRUE(conf.has_group("first"));
++
++    ASSERT_TRUE(conf.remove_key("first", "stringvalue"));
++    EXPECT_THROW(conf.remove_key("first", "missingvalue"), LogicException);
++
++    ASSERT_FALSE(conf.has_key("first", "stringvalue"));
++    ASSERT_TRUE(conf.has_group("first"));
++
++    ASSERT_TRUE(conf.remove_group("first"));
++    EXPECT_THROW(conf.remove_group("nonexisting"), LogicException);
++
++    EXPECT_THROW(conf.has_key("first", "stringvalue"), LogicException);
++    ASSERT_FALSE(conf.has_group("first"));
+ }
  TEST(IniParser, simpleQueries)
@@ -64,11 +77,13 @@
      ASSERT_EQ("second", groups[1]);
      vector<string> firstKeys = conf.get_keys("first");
--    ASSERT_EQ(10, firstKeys.size());
--    ASSERT_EQ("boolvalue", firstKeys[1]);
++    ASSERT_EQ(11, firstKeys.size());
++    ASSERT_EQ("boolvalue", firstKeys[2]);
      ASSERT_EQ(conf.get_string("first", "stringvalue"), "hello");
      ASSERT_EQ(1, conf.get_int("first", "intvalue"));
++    ASSERT_EQ(1, conf.get_double("first", "intvalue"));
++    ASSERT_EQ(2.345, conf.get_double("first", "doublevalue"));
      ASSERT_FALSE(conf.get_boolean("second", "boolvalue"));
      ASSERT_EQ("world", conf.get_string("first", "locstring"));
@@ -92,6 +107,18 @@
      ASSERT_EQ(4, intArr[0]);
      ASSERT_EQ(3, intArr[8]);
++    vector<double> intToDblArr = conf.get_double_array("second", "intarray");
++    ASSERT_EQ(9, intToDblArr.size());
++    ASSERT_EQ(4, intToDblArr[0]);
++    ASSERT_EQ(3, intToDblArr[8]);
++
++    vector<double> dblArr = conf.get_double_array("second", "doublearray");
++    ASSERT_EQ(4, dblArr.size());
++    ASSERT_EQ(4.5, dblArr[0]);
++    ASSERT_EQ(6.78, dblArr[1]);
++    ASSERT_EQ(9, dblArr[2]);
++    ASSERT_EQ(10.11, dblArr[3]);
++
      vector<bool> boolArr = conf.get_boolean_array("first", "boolarray");
      ASSERT_EQ(3, boolArr.size());
      ASSERT_TRUE(boolArr[0]);
@@ -115,36 +142,263 @@
+ {
      IniParser conf(INI_FILE);
--    try {
++    try
++    {
          conf.get_string("foo", "bar");
--        FAIL();
--    } catch(const LogicException &e) {
--    }
--    try {
++    }
++    catch (const LogicException& e)
++    {
++        EXPECT_NE(string::npos, string(e.what()).find("Key file does not have group 'foo'"));
++    }
++    try
++    {
          conf.get_locale_string("foo", "bar");
--        FAIL();
--    } catch(const LogicException &e) {
--    }
--    try {
++    }
++    catch (const LogicException& e)
++    {
++        EXPECT_NE(string::npos, string(e.what()).find("Key file does not have group 'foo'"));
++    }
++    try
++    {
          conf.get_int("foo", "bar");
--        FAIL();
--    } catch(const LogicException &e) {
--    }
--    try {
++    }
++    catch (const LogicException& e)
++    {
++        EXPECT_NE(string::npos, string(e.what()).find("Key file does not have group 'foo'"));
++    }
++    try
++    {
++        conf.get_int("first", "doublevalue");
++    }
++    catch (const LogicException& e)
++    {
++        EXPECT_NE(string::npos, string(e.what()).find("Key file contains key 'doublevalue' in group 'first' which has a value that cannot be interpreted."));
++    }
++    try
++    {
          conf.get_boolean("foo", "bar");
--        FAIL();
--    } catch(const LogicException &e) {
--    }
--
--    try {
++    }
++    catch (const LogicException& e)
++    {
++        EXPECT_NE(string::npos, string(e.what()).find("Key file does not have group 'foo'"));
++    }
++    try
++    {
          conf.get_int_array("first", "array");
--        FAIL();
--    } catch(const LogicException &e) {
--    }
--
--    try {
++    }
++    catch (const LogicException& e)
++    {
++        EXPECT_NE(string::npos, string(e.what()).find("Value 'foo' cannot be interpreted as a number."));
++    }
++    try
++    {
++        conf.get_int_array("second", "doublearray");
++    }
++    catch (const LogicException& e)
++    {
++        EXPECT_NE(string::npos, string(e.what()).find("Value '4.5' cannot be interpreted as a number."));
++    }
++    try
++    {
          conf.get_boolean_array("first", "array");
--        FAIL();
--    } catch(const LogicException &e) {
--    }
++    }
++    catch (const LogicException& e)
++    {
++        EXPECT_NE(string::npos, string(e.what()).find("Value 'foo' cannot be interpreted as a boolean."));
++    }
++}
++
++TEST(IniParser, writeValues)
++{
++    // Create an empty ini file for writing
++    auto f = fopen(INI_TEMP_FILE, "w");
++    fclose(f);
++
++    IniParser conf(INI_TEMP_FILE);
++
++    conf.set_string("g1", "k1", "s0;s1");
++    conf.set_locale_string("g1", "k2", "s2", "en");
++    conf.set_locale_string("g1", "k2", "s3", "pt_BR");
++
++    // Check temp file before sync()
++    {
++        IniParser conf2(INI_TEMP_FILE);
++        EXPECT_EQ("s0;s1", conf.get_string("g1", "k1"));
++        EXPECT_THROW(conf2.get_string("g1", "k1"), LogicException);
++
++        EXPECT_EQ("s2", conf.get_locale_string("g1", "k2", "en"));
++        EXPECT_THROW(conf2.get_locale_string("g1", "k2", "en"), LogicException);
++
++        EXPECT_EQ("s3", conf.get_locale_string("g1", "k2", "pt_BR"));
++        EXPECT_THROW(conf2.get_locale_string("g1", "k2", "pt_BR"), LogicException);
++    }
++
++    // Sync
++    conf.sync();
++
++    // Write some post-sync values
++    conf.set_boolean("g2", "k1", false);
++    conf.set_int("g2", "k2", 123);
++    conf.set_double("g3", "k3", 4.56789);
++
++    // Check temp file after first sync()
++    {
++        IniParser conf2(INI_TEMP_FILE);
++        EXPECT_EQ("s0;s1", conf.get_string("g1", "k1"));
++        EXPECT_EQ("s0;s1", conf2.get_string("g1", "k1"));
++
++        EXPECT_EQ("s2", conf.get_locale_string("g1", "k2", "en"));
++        EXPECT_EQ("s2", conf2.get_locale_string("g1", "k2", "en"));
++
++        EXPECT_EQ("s3", conf.get_locale_string("g1", "k2", "pt_BR"));
++        EXPECT_EQ("s3", conf2.get_locale_string("g1", "k2", "pt_BR"));
++
++        EXPECT_EQ(false, conf.get_boolean("g2", "k1"));
++        EXPECT_THROW(conf2.get_boolean("g2", "k1"), LogicException);
++
++        EXPECT_EQ(123, conf.get_int("g2", "k2"));
++        EXPECT_THROW(conf2.get_int("g2", "k2"), LogicException);
++
++        EXPECT_EQ(4.56789, conf.get_double("g3", "k3"));
++        EXPECT_THROW(conf2.get_double("g3", "k3"), LogicException);
++    }
++
++    // Sync
++    conf.sync();
++
++    // Check temp file after second sync()
++    {
++        IniParser conf2(INI_TEMP_FILE);
++        EXPECT_EQ("s0;s1", conf.get_string("g1", "k1"));
++        EXPECT_EQ("s0;s1", conf2.get_string("g1", "k1"));
++
++        EXPECT_EQ("s2", conf.get_locale_string("g1", "k2", "en"));
++        EXPECT_EQ("s2", conf2.get_locale_string("g1", "k2", "en"));
++
++        EXPECT_EQ("s3", conf.get_locale_string("g1", "k2", "pt_BR"));
++        EXPECT_EQ("s3", conf2.get_locale_string("g1", "k2", "pt_BR"));
++
++        EXPECT_EQ(false, conf.get_boolean("g2", "k1"));
++        EXPECT_EQ(false, conf2.get_boolean("g2", "k1"));
++
++        EXPECT_EQ(123, conf.get_int("g2", "k2"));
++        EXPECT_EQ(123, conf2.get_int("g2", "k2"));
++
++        EXPECT_EQ(4.56789, conf.get_double("g3", "k3"));
++        EXPECT_EQ(4.56789, conf2.get_double("g3", "k3"));
++    }
++}
++
++template<class T>
++void EXPECT_ARRAY_EQ(const vector<T>& expected, const vector<T>& actual)
++{
++    EXPECT_TRUE(expected == actual);
++}
++
++TEST(IniParser, writeArrays)
++{
++    // Create an empty ini file for writing
++    auto f = fopen(INI_TEMP_FILE, "w");
++    fclose(f);
++
++    IniParser conf(INI_TEMP_FILE);
++
++    conf.set_string_array("g1", "k1", {"s0;s1", "s2", "s3"});
++    conf.set_locale_string_array("g1", "k2", {"s4", "s5", "s6"}, "en");
++    conf.set_locale_string_array("g1", "k2", {"s7", "s8", "s9=s10"}, "pt_BR");
++
++    // Check temp file before sync()
++    {
++        IniParser conf2(INI_TEMP_FILE);
++        EXPECT_ARRAY_EQ({"s0;s1", "s2", "s3"}, conf.get_string_array("g1", "k1"));
++        EXPECT_THROW(conf2.get_string_array("g1", "k1"), LogicException);
++
++        EXPECT_ARRAY_EQ({"s4", "s5", "s6"}, conf.get_locale_string_array("g1", "k2", "en"));
++        EXPECT_THROW(conf2.get_locale_string_array("g1", "k2", "en"), LogicException);
++
++        EXPECT_ARRAY_EQ({"s7", "s8", "s9=s10"}, conf.get_locale_string_array("g1", "k2", "pt_BR"));
++        EXPECT_THROW(conf2.get_locale_string_array("g1", "k2", "pt_BR"), LogicException);
++    }
++
++    // Sync
++    conf.sync();
++
++    // Write some post-sync values
++    conf.set_boolean_array("g2", "k1", {true, false, false});
++    conf.set_int_array("g2", "k2", {123, 456789, 101112});
++    conf.set_double_array("g2", "k3", {13.14, 15.161718, 19.2});
++
++    // Check temp file after first sync()
++    {
++        IniParser conf2(INI_TEMP_FILE);
++        EXPECT_ARRAY_EQ({"s0;s1", "s2", "s3"}, conf.get_string_array("g1", "k1"));
++        EXPECT_ARRAY_EQ({"s0;s1", "s2", "s3"}, conf2.get_string_array("g1", "k1"));
++
++        EXPECT_ARRAY_EQ({"s4", "s5", "s6"}, conf.get_locale_string_array("g1", "k2", "en"));
++        EXPECT_ARRAY_EQ({"s4", "s5", "s6"}, conf2.get_locale_string_array("g1", "k2", "en"));
++
++        EXPECT_ARRAY_EQ({"s7", "s8", "s9=s10"}, conf.get_locale_string_array("g1", "k2", "pt_BR"));
++        EXPECT_ARRAY_EQ({"s7", "s8", "s9=s10"}, conf2.get_locale_string_array("g1", "k2", "pt_BR"));
++
++        EXPECT_ARRAY_EQ({true, false, false}, conf.get_boolean_array("g2", "k1"));
++        EXPECT_THROW(conf2.get_boolean_array("g2", "k1"), LogicException);
++
++        EXPECT_ARRAY_EQ({123, 456789, 101112}, conf.get_int_array("g2", "k2"));
++        EXPECT_THROW(conf2.get_int_array("g2", "k2"), LogicException);
++
++        EXPECT_ARRAY_EQ({13.14, 15.161718, 19.2}, conf.get_double_array("g2", "k3"));
++        EXPECT_THROW(conf2.get_double_array("g2", "k3"), LogicException);
++    }
++
++    // Sync
++    conf.sync();
++
++    // Check temp file after second sync()
++    {
++        IniParser conf2(INI_TEMP_FILE);
++        EXPECT_ARRAY_EQ({"s0;s1", "s2", "s3"}, conf.get_string_array("g1", "k1"));
++        EXPECT_ARRAY_EQ({"s0;s1", "s2", "s3"}, conf2.get_string_array("g1", "k1"));
++
++        EXPECT_ARRAY_EQ({"s4", "s5", "s6"}, conf.get_locale_string_array("g1", "k2", "en"));
++        EXPECT_ARRAY_EQ({"s4", "s5", "s6"}, conf2.get_locale_string_array("g1", "k2", "en"));
++
++        EXPECT_ARRAY_EQ({"s7", "s8", "s9=s10"}, conf.get_locale_string_array("g1", "k2", "pt_BR"));
++        EXPECT_ARRAY_EQ({"s7", "s8", "s9=s10"}, conf2.get_locale_string_array("g1", "k2", "pt_BR"));
++
++        EXPECT_ARRAY_EQ({true, false, false}, conf.get_boolean_array("g2", "k1"));
++        EXPECT_ARRAY_EQ({true, false, false}, conf2.get_boolean_array("g2", "k1"));
++
++        EXPECT_ARRAY_EQ({123, 456789, 101112}, conf.get_int_array("g2", "k2"));
++        EXPECT_ARRAY_EQ({123, 456789, 101112}, conf2.get_int_array("g2", "k2"));
++
++        EXPECT_ARRAY_EQ({13.14, 15.161718, 19.2}, conf.get_double_array("g2", "k3"));
++        EXPECT_ARRAY_EQ({13.14, 15.161718, 19.2}, conf2.get_double_array("g2", "k3"));
++    }
++}
++
++TEST(IniParser, writeError)
++{
++    // Create an empty ini file for writing
++    auto f = fopen(INI_TEMP_FILE, "w");
++    fclose(f);
++
++    IniParser conf(INI_TEMP_FILE);
++
++    // Replace ini file with a directory
++    ASSERT_EQ(0, remove(INI_TEMP_FILE));
++    ASSERT_EQ(0, mkdir(INI_TEMP_FILE, 0700));
++
++    std::shared_ptr<void> rmdir_raii(nullptr, [](void*)
++    {
++        rmdir(INI_TEMP_FILE);
++    });
++
++    // Sync (no exception as config is not dirty)
++    EXPECT_NO_THROW(conf.sync());
++
++    // Dirty the config
++    conf.set_boolean("g1", "k1", true);
++
++    // Sync (exception as target is a directory)
++    EXPECT_THROW(conf.sync(), FileException);
+ }