Hi!

Here is a new version that contains the sfidl changes only.

On Wed, Nov 15, 2006 at 02:36:12AM +0100, Tim Janik wrote:
> > Index: sfi/sfidl-parser.cc
> > ===================================================================
> > --- sfi/sfidl-parser.cc     (revision 4015)
> > +++ sfi/sfidl-parser.cc     (working copy)
> > @@ -37,6 +37,14 @@ Sfidl::string_from_int (long long int ll
> >   return str;
> > }
> >
> > +// we don't need that (yet) in the Sfidl namespace
> > +static std::string
> > +string_from_double (double value)
> > +{
> > +  char buffer[G_ASCII_DTOSTR_BUF_SIZE + 1] = "";
> > +  g_ascii_formatd (buffer, G_ASCII_DTOSTR_BUF_SIZE, "%.17e", value);
> > +  return buffer;
> > +}
> 
> there is a Birnet funciton named like this already, re-using that name
> just makes the below code highly confusing (because it's hard to tell
> at which place which funciton gets called).
> either the above fix should go into the birnet variant, or the above
> function should be renamed.

I renamed the function. For those cases where my functions where just
reimplementing birnet variants, I am using those now.

> > namespace {
> > using namespace Sfidl;
> > @@ -997,16 +1005,13 @@ GTokenType Parser::parseStringOrConst (s
> >       if (ci->name == s &&
> >               ci->type != Constant::tIdent)     /* identifier constants 
> > can't be expanded to strings */
> >         {
> > -         char *x = 0;
> >           switch (ci->type)
> >             {
> >               case Constant::tInt:
> > -               s = x = g_strdup_printf ("%lldLL", ci->i);
> > -               g_free (x);
> > +               s = string_from_int (ci->i) + "LL";
> >                 break;
> >               case Constant::tFloat:
> > -               s = x = g_strdup_printf ("%.17g", ci->f);
> > -               g_free (x);
> > +               s = string_from_double (ci->f);
> >                 break;
> >               case Constant::tString:
> >                 s = ci->str;
> > @@ -1522,13 +1527,13 @@ GTokenType Parser::parseParamHints (Para
> >   while (!g_scanner_eof (scanner) && bracelevel > 0)
> >     {
> >       GTokenType t = scanner_get_next_token_with_colon_identifiers 
> > (scanner);
> > -      gchar *token_as_string = 0, *x = 0;
> > +      string token_as_string;
> >       bool current_arg_complete = false;
> > +      char *tmp = 0;
> 
> telling from the use below, declaring tmp without initialization
> could help the compiler catch programming errors.
> 
> also, it should be moved into the innermost scope (that is a general rule
> we apply), i.e. inside the switch {} or case {} braces.

Ok.

> >       if(int(t) > 0 && int(t) <= 255)
> >     {
> > -     token_as_string = g_new0 (char, 2); /* FIXME: leak */
> > -     token_as_string[0] = char(t);
> > +     token_as_string = char(t);
> 
> we put spaces before parenthesis in function calls.

Ok.

Index: sfi/ChangeLog
===================================================================
--- sfi/ChangeLog       (revision 4138)
+++ sfi/ChangeLog       (working copy)
@@ -1,3 +1,24 @@
+Mon Dec 11 19:19:55 2006  Stefan Westerfeld  <[EMAIL PROTECTED]>
+
+       * sfidl-parser.cc: Don't use g_strdup_printf, but g_ascii_formatd for
+       formatting doubles (locale independancy). Some simplification could be
+       achieved by switching from char* to std::string for some code.
+       Switched double format from "%.17g" to "%.17e". This keeps doubles in
+       param specs like (50.0/100.0) in tact, because the C++ compiler will
+       still recognize that we have doubles here. 
+       The old code would print (50/100), because "%g" omits decimal point
+       and trailing digits for those doubles that can be represented without,
+       so that the result of the division (when evaluated by the C++ compiler)
+       would be 0, instead of the expected 0.5.
+
+       * sfidl-parser.hh:
+       * sfidl-parser.cc:
+       * sfidl-corecxx.cc: Use birnet functions to convert between strings
+       and integers, instead of reimplementing our own version.
+
+       * Makefile.am: Start linking sfidl against birnet, as we use birnet
+       string helper functions now.
+
 Sun Dec 3 13:47:31 2006  Stefan Westerfeld  <[EMAIL PROTECTED]>
 
        * sfidl-clientcxx.cc:
Index: sfi/sfidl-parser.cc
===================================================================
--- sfi/sfidl-parser.cc (revision 4138)
+++ sfi/sfidl-parser.cc (working copy)
@@ -28,20 +28,30 @@
 #include <set>
 #include <stack>
 
-const std::string
-Sfidl::string_from_int (long long int lli)
+/* As opposed to the birnet function string_from_double, we use "%.17e"
+ * (instead "%.17g"). This keeps doubles in param specs like (50.0/100.0) in
+ * tact, because the C++ compiler will still recognize that we have doubles
+ * here. 
+ *
+ * [ "%g" omits decimal point and trailing digits for those doubles that can be
+ * represented without, so that the result of the division (when evaluated by
+ * the C++ compiler) would be 0, instead of the expected 0.5.]
+ */
+static std::string
+string_with_exponent_from_double (double value)
 {
-  gchar *cstr = g_strdup_printf ("%lld", lli);
-  std::string str = cstr;
-  g_free (cstr);
-  return str;
+  char buffer[G_ASCII_DTOSTR_BUF_SIZE + 1] = "";
+  g_ascii_formatd (buffer, G_ASCII_DTOSTR_BUF_SIZE, "%.17e", value);
+  return buffer;
 }
 
-
 namespace {
 using namespace Sfidl;
 using namespace std;
 
+using Birnet::string_from_uint;
+using Birnet::string_from_int;
+
 /* --- variables --- */
 static  GScannerConfig  scanner_config_template = {
   const_cast<gchar *>   /* FIXME: glib should use const gchar* here */
@@ -997,16 +1007,13 @@ GTokenType Parser::parseStringOrConst (s
          if (ci->name == s &&
               ci->type != Constant::tIdent)     /* identifier constants can't 
be expanded to strings */
            {
-             char *x = 0;
              switch (ci->type)
                {
                  case Constant::tInt:
-                   s = x = g_strdup_printf ("%lldLL", ci->i);
-                   g_free (x);
+                   s = string_from_int (ci->i) + "LL";
                    break;
                  case Constant::tFloat:
-                   s = x = g_strdup_printf ("%.17g", ci->f);
-                   g_free (x);
+                   s = string_with_exponent_from_double (ci->f);
                    break;
                  case Constant::tString:
                    s = ci->str;
@@ -1522,13 +1529,12 @@ GTokenType Parser::parseParamHints (Para
   while (!g_scanner_eof (scanner) && bracelevel > 0)
     {
       GTokenType t = scanner_get_next_token_with_colon_identifiers (scanner);
-      gchar *token_as_string = 0, *x = 0;
+      string token_as_string;
       bool current_arg_complete = false;
 
-      if(int(t) > 0 && int(t) <= 255)
+      if (int (t) > 0 && int (t) <= 255)
        {
-         token_as_string = g_new0 (char, 2); /* FIXME: leak */
-         token_as_string[0] = char(t);
+         token_as_string = char (t);
        }
       switch (t)
        {
@@ -1540,13 +1546,16 @@ GTokenType Parser::parseParamHints (Para
          break;
        case ',':                 current_arg_complete = true;
          break;
-       case G_TOKEN_STRING:      x = g_strescape (scanner->value.v_string, 0);
-                                 token_as_string = g_strdup_printf ("\"%s\"", 
x);
-                                 g_free (x);
+       case G_TOKEN_STRING:      
+         {
+           char *tmp = g_strescape (scanner->value.v_string, 0);
+           token_as_string = string ("\"") + tmp + "\"";
+           g_free (tmp);
+         }
          break;
-       case G_TOKEN_INT:         token_as_string = g_strdup_printf ("%lluLL", 
scanner->value.v_int64);
+       case G_TOKEN_INT:         token_as_string = string_from_uint 
(scanner->value.v_int64) + "LL";
          break;
-       case G_TOKEN_FLOAT:       token_as_string = g_strdup_printf ("%.17g", 
scanner->value.v_float);
+       case G_TOKEN_FLOAT:       token_as_string = 
string_with_exponent_from_double (scanner->value.v_float);
          break;
        case G_TOKEN_IDENTIFIER:
           {
@@ -1562,22 +1571,24 @@ GTokenType Parser::parseParamHints (Para
              }
 
             if (ci == constants.end())
-              token_as_string = g_strdup_printf ("%s", 
scanner->value.v_identifier);
+              token_as_string = scanner->value.v_identifier;
             else switch (ci->type)
               {
               case Constant::tInt:
-                token_as_string = g_strdup_printf ("%lldLL", ci->i);
+                token_as_string = string_from_int (ci->i) + "LL";
                 break;
               case Constant::tFloat:
-                token_as_string = g_strdup_printf ("%.17g", ci->f);
+                token_as_string = string_with_exponent_from_double (ci->f);
                 break;
               case Constant::tIdent:
-                token_as_string = g_strdup (ci->str.c_str());
+                token_as_string = ci->str;
                 break;
               case Constant::tString:
-                x = g_strescape (ci->str.c_str(), 0);
-                token_as_string = g_strdup_printf ("\"%s\"", x);
-                g_free (x);
+               {
+                 char *tmp = g_strescape (ci->str.c_str(), 0);
+                 token_as_string = string("\"") + tmp + "\"";
+                 g_free (tmp);
+               }
                 break;
               default:
                 g_assert_not_reached ();
@@ -1586,7 +1597,7 @@ GTokenType Parser::parseParamHints (Para
           }
          break;
        default:
-         if (!token_as_string)
+         if (token_as_string.empty())
            return GTokenType (')');
        }
       if (current_arg_complete)
@@ -1601,16 +1612,15 @@ GTokenType Parser::parseParamHints (Para
            }
          current_arg = "";
        }
-      else if (token_as_string)
+      else if (!token_as_string.empty())
        {
          current_arg += token_as_string;
        }
-      if (token_as_string && bracelevel)
+      if (!token_as_string.empty() && bracelevel)
        {
          if (args != "")
            args += ' ';
          args += token_as_string;
-         g_free (token_as_string);
        }
     }
   def.args = args;
Index: sfi/sfidl-corecxx.cc
===================================================================
--- sfi/sfidl-corecxx.cc        (revision 4138)
+++ sfi/sfidl-corecxx.cc        (working copy)
@@ -31,6 +31,8 @@ namespace {
 using namespace std;
 using namespace Sfidl;
 
+using Birnet::string_from_int;
+
 static const gchar*
 canonify_name (const string& s,
                const char    replace = '-')
Index: sfi/sfidl-parser.hh
===================================================================
--- sfi/sfidl-parser.hh (revision 4138)
+++ sfi/sfidl-parser.hh (working copy)
@@ -28,8 +28,6 @@
 
 namespace Sfidl {
 
-const std::string string_from_int (long long int lli);
-
 /* we implement a get() function since operator[] is not const */
 template<typename Key, typename Value>
 class Map : public std::map<Key,Value> {
Index: sfi/Makefile.am
===================================================================
--- sfi/Makefile.am     (revision 4138)
+++ sfi/Makefile.am     (working copy)
@@ -61,7 +61,7 @@ common_idl_sources = sfidl-generator.cc 
 
 bin_PROGRAMS = sfidl
 sfidl_SOURCES = sfidl.cc $(common_idl_sources)
-sfidl_LDADD = $(SFI_LIBS) -lm # libsfi.la
+sfidl_LDADD = $(SFI_LIBS) -lm $(top_builddir)/birnet/libbirnet.o # libsfi.la
 sfidl_CFLAGS = $(AM_CFLAGS) # hack to cause glib-extra.c to be compiled twice 
(work around automake)
 EXTRA_DIST += sfidl-generator.hh sfidl-namespace.hh sfidl-options.hh 
sfidl-parser.hh sfidl-factory.hh sfidl-typelist.hh
 EXTRA_DIST += sfidl-cbase.hh sfidl-clientc.hh sfidl-clientcxx.hh 
sfidl-corec.hh sfidl-corecxx.hh sfidl-cxxbase.hh sfidl-hostc.hh

It passes

   make all install report

Ok to commit?

   Cu... Stefan
-- 
Stefan Westerfeld, Hamburg/Germany, http://space.twc.de/~stefan
_______________________________________________
beast mailing list
[email protected]
http://mail.gnome.org/mailman/listinfo/beast

Reply via email to