[rfc][icedtea-web] read properties values from C part - library edition :)

Jiri Vanek jvanek at redhat.com
Thu Mar 21 09:41:23 PDT 2013


Here we go.

Included are also Omair's suggestions.


J.
On 03/20/2013 03:45 PM, Adam Domurad wrote:
> On 03/20/2013 10:19 AM, Jiri Vanek wrote:
>> On 03/19/2013 03:44 PM, Adam Domurad wrote:
>>> On 03/19/2013 09:41 AM, Jiri Vanek wrote:
>>>> So another round. I think all is fixed excet the c x c+++ headers. Do you mind t write little bit
>>>> more what you need from me?
>>>>
>>>> Also unittests will be needed. I would like to wrote them, ..will need probably some code snippet
>>>> and general advices from you  when I will integrate (next round?) this "library" inside ITW.
>>>
>>> Doesn't look half bad this time :-) Feel free to propose an integrated version.
>> One more round rather...
>> I have made the header much bigger - for testing purposes. I have some memory that all exported
>> (testable) functions must be in header file...
>
> Note: You do not need these in header, technically. It may be cleaner to not expose these normally
> and include the lines directly in the test, ie:
>
> string trim(string& str);
> void remove_all_spaces(string& str);
>
> SUITE (...) {
>    /// etc
> }
>
> TYVM for reorganizing the code to be testable! I know the elegance is hard to part with.
>
>
> [.. rest snipped ..]
>
>
>> #include <unistd.h>
>> #include <sys/types.h>
>> #include <pwd.h>
>> #include <cstdio>
>> #include <cstdlib>
>> #include <string>
>> #include <functional>
>> #include <cctype>
>> #include <locale>
>> #include <iostream>
>> #include <fstream>
>> #include "IcedTeaParsePropeties.h"
>>
>> using namespace std;
>>
>> string trim(string& str) {
>>     size_t start = str.find_first_not_of(" \t\n"), end = str.find_last_not_of(" \t\n");
>>     if (start == std::string::npos) {
>>             return str;
>>     }
>>     str = str.substr(start, end - start + 1);
>>     return str;
>> }
>
> I don't see how this supports chaining (it returns only a copy). Also you don't use this string
> value anywhere, why not just 'void'?
> I'm in favour of moving this into IcedTeaPluginUtils and dropping the return value.
>
>>
>>
>>
>>
>> void remove_all_spaces(string& str)
>> {
>>     for(int i=0; i<str.length(); i++){
>>         if(str[i] == ' '  || str[i] == '\n' || str[i] == '\t') {
>>             str.erase(i,1);
>>         }
>>     }
>> }
>>
>> bool  get_property_value(string c, string& dest){
>>     int i = c.find("=");
>>     if (i < 0) return false;
>>     int l = c.length();
>>     dest = c.substr(i+1, l-i);
>>     trim(dest);
>>     return true;
>> }
>>
>>
>> bool starts_with(string c1, string c2){
>>         return (c1.find(c2) == 0);
>> }
>>
>> bool file_exists(string filename)
>> {
>>     ifstream infile(filename.c_str());
>>     return infile.good();
>> }
>>
>>
>> string  user_properties_file(){
>>     int myuid = getuid();
>>     struct passwd *mypasswd = getpwuid(myuid);
>>     return string(mypasswd->pw_dir)+"/.icedtea/"+default_file_name;
>> }
>>
>>
>> string  main_properties_file(){
>>     return "/etc/.java/deployment/"+default_file_name;
>> }
>>
>> string default_java_properties_file(){
>>     return  ICEDTEA_WEB_JRE "/lib/"+default_file_name;
>> }
>>
>>
>> /* this is the same search done by icedtea-web settings:
>>    try  the main file in /etc/.java/deployment
>>    if found, then return this file
>>    try to find setUp jre
>>    if found, then try if this file exists and end
>>    if no jre custom jvm is set, then tries default jre
>>    if its  deploy file exists, then return
>>    not found otherwise*/
>> bool find_system_config_file(string& dest){
>>     string jdest;
>>     bool found = find_custom_jre(jdest);
>>     return find_system_config_file(main_properties_file(), jdest, found,
>> default_java_properties_file(), dest);
>> }
>> bool find_system_config_file(string f1, string f2, bool usef2, string f3, string& dest){
>
> f1, f2, f3 -> user_file, global_file, default_file please. Also worth considering making the 2nd
> string nullable, ie using a string* (note this won't be as painful as returning string*, since you
> can safely pass with & operator here).
>
>>     if (file_exists(f1)) {
>>         dest = f1;
>>         return true;
>>     } else {
>>         if (usef2){
>>             string jdest = f2 + "/lib/"+default_file_name;
>>             if(file_exists(jdest) ) {
>>                 dest = jdest;
>>                 return true;
>>             }
>>         } else {
>>             if(file_exists(f3)) {
>>             dest = f3;
>>             return true;
>>             }
>>         }
>>     }
>> return false; //nothing of above found
>> }
>>
>> //Returns whether property was found, if found stores result in 'dest'
>> bool find_property(string filename, string property, string& dest){
>>     string  property_matcher(property);
>>     trim( property_matcher);
>>      property_matcher= property_matcher+"=";
>>     ifstream input( filename.c_str() );
>>     for( string line; getline( input, line ); ){ /* read a line */
>>         string copy = line;
>>         //java tolerates spaces around = char, remove them for matching
>>         remove_all_spaces(copy);
>>         //printf(line.c_str());
>>         //printf(copy.c_str());
>
> [nit] remove these commented lines
>
>>         if (starts_with(copy, property_matcher)) {
>>             //provide non-spaced value, trimming is  done in get_property_value
>>             get_property_value(line, dest);
>>             return true;
>>             }
>>         }
>>
>>     return false;
>>     }
>>
>>
>> /* this is reimplmentation of itw-settings operations
>
> OK. First time you typo something, turn on some spellcheck >:).
> s/reimplmentation/reimplementation/ ... again
>
>>    first check in user's settings, if found, return
>>    then check in global file (see the magic of find_system_config_file)*/
>> bool  read_deploy_property_value(string property, string& dest){
>>     string futurefile;
>>     bool b = find_system_config_file(futurefile);
>>     return read_deploy_property_value(user_properties_file(), futurefile, b, property, dest);
>> }
>> bool  read_deploy_property_value(string f1, string f2, bool usef2, string property, string& dest){
>
> f1, f2 -> user_file, global_file please. Consider making global_file a nullable string*.
>
>>     //is it in user's file?
>>     bool found = find_property(f1, property, dest);
>>     if (found) {
>>         return true;
>>     }
>>     //is it in global file?
>>     if (usef2) {
>>         return find_property(f2, property, dest);
>>     }
>>     return false;
>> }
>>
>> //This is different from common get property, as it is avoiding to search in *java*
>> //properties files
>> bool  find_custom_jre(string& dest){
>>     return find_custom_jre(user_properties_file(), main_properties_file(), dest);
>> }
>> bool  find_custom_jre(string f1, string f2,string& dest){
>
> f1, f2 -> user_file, global_file please
>
>>     string key = custom_jre_key;
>>     if(file_exists(f1)) {
>>         bool a = find_property(f1, key, dest);
>>         if (a) {
>>             return true;
>>         }
>>     }
>>     if(file_exists(f2)) {
>>         return find_property(f2, key, dest);
>>     }
>> return false;
>> }
>>
>>
>>
>> int main(void){
>>         printf("user's settings file\n");
>>     cout << user_properties_file();
>>     printf("\nmain settings file:\n");
>>     cout << (main_properties_file());
>>     printf("\njava settings file \n");
>>     cout << (default_java_properties_file());
>>     printf("\nsystem config file\n");
>>     string a1;
>>     find_system_config_file(a1);
>>     cout <<  a1;
>>     printf("\ncustom jre\n");
>>     string a2;
>>     find_custom_jre(a2);
>>     cout << a2;
>>     printf("\nsome custom property\n");
>>     string a3;
>>     read_deploy_property_value("deployment.security.level", a3);
>>     cout << a3;
>>     printf("\n");
>>   return 0;
>> }
>
> Looks good otherwise, go ahead and start testing+integration if you'd like.
> -Adam

-------------- next part --------------
A non-text attachment was scrubbed...
Name: integratedAndTestedPArseproperties.patch
Type: text/x-patch
Size: 29152 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20130321/d8891f0a/integratedAndTestedPArseproperties.patch 


More information about the distro-pkg-dev mailing list