firebird-devel

[Firebird-devel] The last of the "no effect" warning/error fixes.

[Mark O'D. <mar...@lu...>]

Here are some more changes for the "no effects" warning removals.


in gpre/sqe.c

From:

if (conflict)
    {
    SCHAR *error_type;
    if (symbol->sym_type == SYM_relation)
        error_type == "table";
    else if (symbol->sym_type == SYM_procedure)
        error_type == "procedure";
    else
        error_type == "context";


Changed to:

if (conflict)
    {
    SCHAR *error_type;
    if (symbol->sym_type == SYM_relation)
        error_type = "table";
    else if (symbol->sym_type == SYM_procedure)
        error_type = "procedure";
    else
        error_type = "context";


Another occurance of the == where a = was required.



In burp/backup.e

The following code occurs in two places:

                   STUFF (name_len);
                   for (; c = *name++, name_len--;  name_len)
                       STUFF (c);

Obvioulsy the  ; name_len) does nothing.

Changed to:

                   STUFF (name_len);
                   for (; c = *name++, name_len--;)
                       STUFF (c);


That has to be a cute change one that results in a ;) in the code.



Another in burp/backup.e

        for (rp = field->fld_ranges, n = field->fld_dimensions; *rp, n;
rp+=2, n--)
            {
            PUT_NUMERIC (att_field_range_low, *rp);
            PUT_NUMERIC (att_field_range_high, *(rp+1));
            }
        }

Changed to:

        for (rp = field->fld_ranges, n = field->fld_dimensions;  n;
rp+=2, n--)
        ...

See seperate email about this one for choice of n over *rp && n.


In burp/burp.c

            for (p = redirect, string = OUTPUT_SUPPRESS,
tdgbl->sw_redirect = NOOUTPUT; c = *p++; *string)
                {
                if (UPPER (c) != *string++)
                    {
                    tdgbl->sw_redirect = TRUE;
                    break;
                    }
                }


Changed to:

 for (p = redirect, string = OUTPUT_SUPPRESS, tdgbl->sw_redirect =
NOOUTPUT; c = *p++; )

Removed the *string from the for increment section, it look like someone
moved the *string++ into the code but forgot to completely remove it
from the for increment.



In dudley/parse.c

static FUNCARG parse_function_arg (
    FUNC        function,
    USHORT      *position)
{
...

KEYWORD (KW_BY);
LEX_token();



The KEYWORD macro expands to :
#define KEYWORD(kw)             (DDL_token.tok_keyword == kw)

So it does nothing and generates a warning.  I've commented it out for
now, since it is the old gdml code, but the correct change may be to
make it something based on:

if (!KEYWORD(KW_BY)) {
    PARSE_error(....)
}

I'll wait for some feedback.


In isql/isql.e

void ISQL_warning (    STATUS      *status) {
...
status[2] == gds_arg_end;


Changed to:

status[2] = gds_arg_end;

Which seems fairly clearly the intention.


In qli/meta.e

The statement
length;
doesn't add a lot of value in the function and generates a warning, I
removed it.
in:

static SYM make_symbol (    TEXT        *string,   SSHORT      length) {

SYM     symbol;
TEXT    *p, *end;;

if (!(length = truncate_string (string)))
    return NULL;

length;

symbol = (SYM) ALLOCPV (type_sym, length);
symbol->sym_type = SYM_relation;
symbol->sym_length = length;
symbol->sym_string = p = symbol->sym_name;
do *p++ = *string++; while (--length);

return symbol;
}



Ok Thats it no "no effects" warnings left (except the one in alice.c to
follow up on).

But tell you what I'll be pissed off if Inprise take just this and say
"thanks sucker" just another free developer to add value to an Inprise
product.

Cheers

Mark

Re: [Firebird-devel] The last of the "no effect" warning/error fixes.

[Geoffrey C. S. <ge...@se...>]

On Sat, 26 Aug 2000, Mark O'Donohue wrote:

> In burp/backup.e
> 
>                    STUFF (name_len);
>                    for (; c = *name++, name_len--;)
>                        STUFF (c);

I personally don't ever see the need for anything of the form

	for (; statement ;)

when using a nice 'while' will yeild less semicolon clutter, but then I
suppose I don't find 'for (; statement ;)' as cute as some people.  ;)

> Another in burp/backup.e
> 
> Changed to:
> 
>         for (rp = field->fld_ranges, n = field->fld_dimensions;  n;
> rp+=2, n--)

After just glancing at this (and the other posting), the change looks
good.  It appears that the semantics of fld_dimensions is the length of
fld_ranges.  The two lines from restore.e really cinched that (and are
much easier to look at):

            end_ranges = fld_ranges + 2 * field->fld_dimensions;
            for (range = fld_ranges; range < end_ranges; range += 2)

Keep up the good work.  I'll contribute more after September comes around,
when I'm not spending 12 hours a day in the office.  :)

Geoff

-- 
Geoffrey C. Speicher                     Tel: (570)803-0535 x1701
ge...@se...               Fax: (570)803-0536
Software Engineering Associates, Inc.    http://www.sea-incorporated.com/

Re: [Firebird-devel] The last of the "no effect" warning/error fixes.

[Ann H. <har...@ne...>]

At 05:24 PM 8/26/00 +1000, Mark O'Donohue wrote:


>Here are some more changes for the "no effects" warning removals.

>         error_type == "table";
>         error_type == "procedure";
>         error_type == "context";

The compiler we used when I was learning C didn't report a warning
for that sort of idiocy.  After I'd been working in gpre for about
six months, Jim found that error in something I'd coded - gave me
public hell about it (everything was public - we were a two room
company and the walls were tissue paper).  He insisted I go over
every piece of code I had written and check that I had assignment
and comparison correct.  Actually, thinking about it, I'd done
the opposite - assigned where I should have compared.

>In burp/backup.e
>
>The following code occurs in two places:
>
>                    STUFF (name_len);
>                    for (; c = *name++, name_len--;  name_len)
>                        STUFF (c);
>
>Obvioulsy the  ; name_len) does nothing.
>
>Changed to:
>
>                    STUFF (name_len);
>                    for (; c = *name++, name_len--;)
>                        STUFF (c);

My preference, when a for loop degenerates to its exit
condition is to change loop types.



>In burp/burp.c
>
>             for (p = redirect, string = OUTPUT_SUPPRESS,
>tdgbl->sw_redirect = NOOUTPUT; c = *p++; *string)
>                 {
>                 if (UPPER (c) != *string++)
>                     {
>                     tdgbl->sw_redirect = TRUE;
>                     break;
>                     }
>                 }
>
>
>Changed to:
>
>  for (p = redirect, string = OUTPUT_SUPPRESS, tdgbl->sw_redirect =
>NOOUTPUT; c = *p++; )

Again, I'd put the two setup statements outside the loop and
have something clear like
            while (c = *p++)


>In dudley/parse.c
>
>static FUNCARG parse_function_arg (
>     FUNC        function,
>     USHORT      *position)
>{
>...
>
>KEYWORD (KW_BY);
>LEX_token();

The lexeme "by" is used in the GDL (groton data language) in
two ways:  a column can be COMPUTED [BY] an expression and a
function parameter can be passed [BY] {VALUE | REFERENCE ...}

The wrong macro is used here.  It should be MATCH (KW_BY) which
is the way to skip a noise word.

>In isql/isql.e
>
>void ISQL_warning (    STATUS      *status) {
>...
>         status[2] == gds_arg_end;
>Changed to:
>         status[2] = gds_arg_end;

Exactly.

>In qli/meta.e
>
>The statement
>length;
>doesn't add a lot of value in the function  make_symbol


>But tell you what I'll be pissed off if Inprise take just this and say
>"thanks sucker" just another free developer to add value to an Inprise
>product.

You expect thanks from them?

Ann

Re: [Firebird-devel] The last of the "no effect" warning/errorfixes.

[Mark O'D. <mar...@lu...>]

Ann Harrison wrote:

> At 05:24 PM 8/26/00 +1000, Mark O'Donohue wrote:
>
> >
> >Changed to:
> >
> >                    STUFF (name_len);
> >                    for (; c = *name++, name_len--;)
> >                        STUFF (c);
>
> My preference, when a for loop degenerates to its exit
> condition is to change loop types.

I'd agree, and so did Geoff.  I was just being touchy, becuase I haven't
actually coded in pure C for about 6 yrs, and even when I did, I can't
actually remember actually using the "," operator.  I now have reserected
my ancient "A book on C" and "C a Reference manual" (including
draft-proposed ansi standards - so its the newer copy)  and now have them
beside me, although the more I go, the more it IS all comming back to me.

I'll redo them tonight, since we all seem to agree on it.

>
> >In dudley/parse.c
> >
> >static FUNCARG parse_function_arg (
> >     FUNC        function,
> >     USHORT      *position)
> >{
> >...
> >
> >KEYWORD (KW_BY);
> >LEX_token();
>
> The lexeme "by" is used in the GDL (groton data language) in
> two ways:  a column can be COMPUTED [BY] an expression and a
> function parameter can be passed [BY] {VALUE | REFERENCE ...}
>
> The wrong macro is used here.  It should be MATCH (KW_BY) which
> is the way to skip a noise word.

Will also change - thanks, I got a gdml manual section from Helen, but
couldn't find "by" as a keword in the portion that she had given me.


Cheers

Mark

Re: [Firebird-devel] The last of the "no effect" warning/errorfixes.

[Mark O'D. <mar...@lu...>]

Ok I've fixed the ones below as we all agreed.

Have a quick look, remember that It's been 6 years since I seriously did
some C,  Java/C++ - well that's another story.

Like I said, no test suite, (yet) so peer review here counts for a lot.

Cheers

Mark

Ann Harrison wrote:

> At 05:24 PM 8/26/00 +1000, Mark O'Donohue wrote:
>
>
> >In burp/backup.e
> >
> >The following code occurs in two places:
> >
> >                    STUFF (name_len);
> >                    for (; c = *name++, name_len--;  name_len)
> >                        STUFF (c);
> >
> >Obvioulsy the  ; name_len) does nothing.
> >
> Changed to:

                    STUFF (name_len);
                   while(name_len--) {
                      c = *name++;
                      STUFF (c);
                   }


>
> My preference, when a for loop degenerates to its exit
> condition is to change loop types.
>
> >In burp/burp.c
> >
> >             for (p = redirect, string = OUTPUT_SUPPRESS,
> >tdgbl->sw_redirect = NOOUTPUT; c = *p++; *string)
> >                 {
> >                 if (UPPER (c) != *string++)
> >                     {
> >                     tdgbl->sw_redirect = TRUE;
> >                     break;
> >                     }
> >                 }
> >
> >
> >Changed to:
>

            p = redirect;
            string = OUTPUT_SUPPRESS;
            tdgbl->sw_redirect = NOOUTPUT;
            while (c = *p++) {
                if (UPPER (c) != *string++)
                    {
                    tdgbl->sw_redirect = TRUE;
                    break;
                    }
                }


>
> >In dudley/parse.c
> >
> >static FUNCARG parse_function_arg (
> >     FUNC        function,
> >     USHORT      *position)
> >{
> >...
> >
> >KEYWORD (KW_BY);
> >LEX_token();
>
> The lexeme "by" is used in the GDL (groton data language) in
> two ways:  a column can be COMPUTED [BY] an expression and a
> function parameter can be passed [BY] {VALUE | REFERENCE ...}
>
> The wrong macro is used here.  It should be MATCH (KW_BY) which
> is the way to skip a noise word.
>

Done.

>
> >In isql/isql.e
> >
> >void ISQL_warning (    STATUS      *status) {
> >...
> >         status[2] == gds_arg_end;
> >Changed to:
> >         status[2] = gds_arg_end;
>
> Exactly.
>
> >In qli/meta.e
> >
> >The statement
> >length;
> >doesn't add a lot of value in the function  make_symbol
>

Re: [Firebird-devel] The last of the "no effect" warning/errorfixes.

[Mike N. <ta...@al...>]

Mark O'Donohue wrote:
> > >In burp/burp.c
> > >
> > >             for (p = redirect, string = OUTPUT_SUPPRESS,
> > >tdgbl->sw_redirect = NOOUTPUT; c = *p++; *string)
> > >                 {
> > >                 if (UPPER (c) != *string++)
> > >                     {
> > >                     tdgbl->sw_redirect = TRUE;
> > >                     break;
> > >                     }
> > >                 }
> > >
> > >
> > >Changed to:
> >
> 
>             p = redirect;
>             string = OUTPUT_SUPPRESS;
>             tdgbl->sw_redirect = NOOUTPUT;
>             while (c = *p++) {
>                 if (UPPER (c) != *string++)
>                     {
>                     tdgbl->sw_redirect = TRUE;
>                     break;
>                     }
>                 }

What about

    tdgbl->sw_redirect =
        (redirect && STRICMP(redirect, OUTPUT_SUPPRESS)) ? TRUE : NOOUTPUT;

?

/Mike

[Firebird-devel] UDF's

[Geoffrey C. S. <ge...@se...>]

I'm trying to figure out if the FreeBSD port is broken w.r.t. UDF's or if
I'm just plain doing something wrong.

if I have a my_udf.c like this (adapted from examples/udflib.c):

--snip--
#include <stdio.h>
#include <stdlib.h>
#include <ibase.h>

char* my_lower (char* s)
{
    char *buf;
    short length = 0;

    char *buffer = (char *)malloc(256);

    length = (short)*s;
    s += 2;
    buf = buffer;
    while (*s)
        if (*s >= 'A' && *s <= 'Z')
            *buf++ = *s++ - 'A' + 'a';
        else
            *buf++ = *s++;

    *buf = '\0';
    buffer [length] = '\0';

    return buffer;
}
--snip--

and I compile/link it with this:

cc -c -o my_udf.o -I/usr/interbase/include my_udf.c
ld -shared -o my_udf.so my_udf.o

and I put my_udf.so in /usr/interbase/UDF and chmod 755 it,

and I declare an external function like this (adapted from
examples/udf.sql):

DECLARE EXTERNAL FUNCTION my_lower 
        VARCHAR (256)   
        RETURNS CSTRING(80) FREE_IT
        ENTRY_POINT 'my_lower' MODULE_NAME 'my_udf.so';

and I try to use it like this:

SQL> select my_lower(gpaid) from gpa;

and I get this error:

Statement failed, SQLCODE = -104

invalid request BLR at offset 63
-function MY_LOWER is not defined
-module name or entrypoint could not be found

then what am I doing wrong?  I don't think the port is broken, because
this works with the stock ib_udf library:

DECLARE EXTERNAL FUNCTION lower 
        CSTRING(80)
        RETURNS CSTRING(80) FREE_IT
        ENTRY_POINT 'IB_UDF_lower' MODULE_NAME 'ib_udf';

But I'm really at a loss for why I can't use my own module.  I've tried
moving my_udf.so to /usr/interbase/lib and /usr/local/lib with no luck.

Anyone?

Geoff

-- 
Geoffrey C. Speicher                     Tel: (570)803-0535 x1701
ge...@se...               Fax: (570)803-0536
Software Engineering Associates, Inc.    http://www.sea-incorporated.com/

Re: [Firebird-devel] UDF's

[reed m. <rf...@cr...>]

In 6.0, udfs must be in a specific directory (called UDF, I
think, but not sure) If you think about the consequences
of declaring, for example 'system' in libc.so as a UDF, you
will see why it is restricted this way ;-(
All versions prior to 6.0 are vulnerable to this.

The relavent code should be jrd/flu.c

"Geoffrey C. Speicher" wrote:
> 
> I'm trying to figure out if the FreeBSD port is broken w.r.t. UDF's or if
> I'm just plain doing something wrong.
> 
> if I have a my_udf.c like this (adapted from examples/udflib.c):
> 
> --snip--
> #include <stdio.h>
> #include <stdlib.h>
> #include <ibase.h>
> 
> char* my_lower (char* s)
> {
>     char *buf;
>     short length = 0;
> 
>     char *buffer = (char *)malloc(256);
> 
>     length = (short)*s;
>     s += 2;
>     buf = buffer;
>     while (*s)
>         if (*s >= 'A' && *s <= 'Z')
>             *buf++ = *s++ - 'A' + 'a';
>         else
>             *buf++ = *s++;
> 
>     *buf = '\0';
>     buffer [length] = '\0';
> 
>     return buffer;
> }
> --snip--
> 
> and I compile/link it with this:
> 
> cc -c -o my_udf.o -I/usr/interbase/include my_udf.c
> ld -shared -o my_udf.so my_udf.o
> 
> and I put my_udf.so in /usr/interbase/UDF and chmod 755 it,
> 
> and I declare an external function like this (adapted from
> examples/udf.sql):
> 
> DECLARE EXTERNAL FUNCTION my_lower
>         VARCHAR (256)
>         RETURNS CSTRING(80) FREE_IT
>         ENTRY_POINT 'my_lower' MODULE_NAME 'my_udf.so';
> 
> and I try to use it like this:
> 
> SQL> select my_lower(gpaid) from gpa;
> 
> and I get this error:
> 
> Statement failed, SQLCODE = -104
> 
> invalid request BLR at offset 63
> -function MY_LOWER is not defined
> -module name or entrypoint could not be found
> 
> then what am I doing wrong?  I don't think the port is broken, because
> this works with the stock ib_udf library:
> 
> DECLARE EXTERNAL FUNCTION lower
>         CSTRING(80)
>         RETURNS CSTRING(80) FREE_IT
>         ENTRY_POINT 'IB_UDF_lower' MODULE_NAME 'ib_udf';
> 
> But I'm really at a loss for why I can't use my own module.  I've tried
> moving my_udf.so to /usr/interbase/lib and /usr/local/lib with no luck.
> 
> Anyone?
> 
> Geoff
> 
> --
> Geoffrey C. Speicher                     Tel: (570)803-0535 x1701
> ge...@se...               Fax: (570)803-0536
> Software Engineering Associates, Inc.    http://www.sea-incorporated.com/
> 
> _______________________________________________
> Firebird-devel mailing list
> Fir...@li...
> http://lists.sourceforge.net/mailman/listinfo/firebird-devel


--
Reed Mideke                  rfm(at)cruzers.com
If that doesn't work:        rfm(at)portalofevil.com 
InterBase build instructions: www.cruzers.com/~rfm

Re: [Firebird-devel] UDF's

[Geoffrey C. S. <ge...@se...>]

On Tue, 29 Aug 2000, reed mideke wrote:

> In 6.0, udfs must be in a specific directory (called UDF, I
> think, but not sure) If you think about the consequences
> of declaring, for example 'system' in libc.so as a UDF, you
> will see why it is restricted this way ;-(
> All versions prior to 6.0 are vulnerable to this.
> 
> The relavent code should be jrd/flu.c

Yep, it's there alright.  It confirms what you said and what I read in
documentation about the library having to be in interbase/UDF/ or in a
directory listed in interbase/isc_config.

Unfortunately, still the only library that is working for me is ib_udf.  I
guess it must be something in how I'm building my udf library.

Geoff

-- 
Geoffrey C. Speicher                     Tel: (570)803-0535 x1701
ge...@se...               Fax: (570)803-0536
Software Engineering Associates, Inc.    http://www.sea-incorporated.com/

Re: [Firebird-devel] UDF's

[Geoffrey C. S. <ge...@se...>]

Well, it turns out that I'm working on a FreeBSD 4.1 machine now, and
using the 4.0 binaries that I released earlier.  There was a new version
of binutils, and so the 4.0 IB6 binaries couldn't load the 4.1 shared
library.

I recompiled IB on FreeBSD 4.1 and all is well again.  I ran into that
dsql/parse.c problem again, and resolved it the same way I did last
time---delete the parse.c that my yacc generated and get the original one
back from the CVS server.  Since parse.c is then newer than anything else,
it's not regenerated, and everything works.

So I think this has to be attributed to different versions of yacc.  I bet
you Linux folks are using GNU Bison or something that I'm not.  Also,
check out the target for parse.c in dsql/makefile:

parse.c:        parse.y parse.sed
       $(YACC) -l $(YFLAGS) $<
# replace the above line with the following three for FLINTSTONE
#       awk -f parse.awk FLINTSTONE=1 < $< > tmp.$<
#       $(YACC) -l $(YFLAGS) tmp.$<
#       -$(RM) tmp.$<
        sed -f parse.sed y.tab.c > $@
        -$(RM) y.tab.c

Isn't FLINTSTONE used to indicate a production version of the binaries?

Geoff

-- 
Geoffrey C. Speicher                     Tel: (570)803-0535 x1701
ge...@se...               Fax: (570)803-0536
Software Engineering Associates, Inc.    http://www.sea-incorporated.com/

Re: [Firebird-devel] UDF's

[reed m. <rf...@cr...>]

> So I think this has to be attributed to different versions of yacc.  I bet
> you Linux folks are using GNU Bison or something that I'm not.  Also,
> check out the target for parse.c in dsql/makefile:

I actually just touched parse.c before building (on linux) because
I didn't have a yacc (yeah bison was, there, but I haven't got
around to trying it yet). As long as you are not actually making changes
to parse.y, this should be fine ;-(. Depending what order CVS pulled
them out, other people might not actually be rebuilding it.
Obviously, on NT, we just use the checked in file. I think the checked
in versions at IB were yacced using solaris.

Looking at parse.sed, it seems it might be pretty specific to
the yacc involved.
> 
> parse.c:        parse.y parse.sed
>        $(YACC) -l $(YFLAGS) $<
> # replace the above line with the following three for FLINTSTONE
> #       awk -f parse.awk FLINTSTONE=1 < $< > tmp.$<
> #       $(YACC) -l $(YFLAGS) tmp.$<
> #       -$(RM) tmp.$<
>         sed -f parse.sed y.tab.c > $@
>         -$(RM) y.tab.c
> 
> Isn't FLINTSTONE used to indicate a production version of the binaries?
> 
FLINTSTONE is totally useless. I'm 99.9% positive if you grep
the source for it, you will not find it used ANYWHERE. It was
the code-name for a 4.0 project that somehow never got
removed from the makefiles. 

OK. I just verified this. FLINTSTONE can be removed from the makefiles.
But what is interesting is that FLINT_CACHE still exists both in the
checked-in parse.c and in sql.c
Doesn't look like it is ever defined. We'll have to find someone from
the 4.0 project to tells what this all means.

for d in * ; do if [ -d $d ] ; then grep FLINT $d/*.[ceh]; fi; done
dudley/parse.c:#ifdef FLINT_CACHE
dudley/parse.c:#endif /* FLINT_CACHE */
dudley/parse.c:#ifdef FLINT_CACHE
dudley/parse.c:#endif /* FLINT_CACHE */
gpre/sql.c:#ifdef FLINT_CACHE
gpre/sql.c:#endif /* FLINT_CACHE */
gpre/sql.c:#ifdef FLINT_CACHE
gpre/sql.c:#endif /* FLINT_CACHE */
gpre/sql.c:#ifdef FLINT_CACHE
gpre/sql.c:#endif /* FLINT_CACHE */
gpre/sql.c:#ifdef FLINT_CACHE
gpre/sql.c:#endif /* FLINT_CACHE */
--
Reed Mideke                  rfm(at)cruzers.com
If that doesn't work:        rfm(at)portalofevil.com 
InterBase build instructions: www.cruzers.com/~rfm

[Firebird-devel] Parse.y, parse.c

[Ann H. <har...@ne...>]

For some years, parse.c was created only on Solaris (or was
it HP? ... no, not HP), because that particular yacc accepted
the .y file and others did not.  This spring, we tried a few
other yaccs ... the platforms escape me at the moment ...
cleaned up a few "quirks" in parse.y and had it generally
working.

What I'm trying to say is that parse.y is not guaranteed to
get along with just any yacc.

Ann

Re: [Firebird-devel] UDF's

[Mike N. <ta...@al...>]

reed mideke wrote:
[.c : .y]
> Obviously, on NT, we just use the checked in file.

OK, Win32 don't have hese tools installed "by default" like unices, but I
don't think it's obvious it doesn't exist on a developers machine. I have
ready-to-run yacc and bison on Win32 now, and if I hadn't it would be a
matter of seconds to rebuild any of them.

/Mike