Menu
▾
▴
naviserver-devel
|
From: Gustaf N. <ne...@wu...> - 2012-10-16 09:33:57
|
Dear Maurizio,
First of all, many thanks for your efforts! These are very
welcome!
I am as well somewhat surprised, that there are still pieces
of K&R style around.
There is really no need to keep these. i'll commit an
updated version of tclxkeylist.c
to mercurial. As for the other changes, i am not really
convinced that changing
all variables named "new" to "mm_new" helps to improve the
code...
for the other changes, your archive file is the full kitchen
sink, going though
the changes will take a while.
We should move the discussion to the naviserver-devel list
Best regards
-gustaf neumann
On 16.10.12 00:36, Maurizio Martignano wrote:
>
> To facilitate the discussion, I put in here some examples
> of the changes I made:
>
> *2.a.*
>
> From
>
> mapPtr = ns_malloc(sizeof(Map));
>
> To
>
> mapPtr = (Map*) ns_malloc(sizeof(Map));
>
> *2.b.*
>
> From
>
> int
>
> TclX_WrongArgs (interp, commandNameObj, string)
>
> Tcl_Interp* interp;
>
> Tcl_Obj* commandNameObj;
>
> char * string;
>
> {
>
> To
>
> int
>
> TclX_WrongArgs (Tcl_Interp* interp, Tcl_Obj*
> commandNameObj, char* string)
>
> {
>
> *2.c*
>
> From
>
> if(new) {
>
> To
>
> if(mm_new) {
>
> All these changes have been applied everywhere.
>
> ========
>
> Dear all,
>
> While the tests seem to go rather well but
> are not finished yet (and it will take quite some time), I
> have decided to make available anyhow the result of my
> activity.
>
> The sources (and Visual Studio 2012 project files) are
> available at:
>
> http://www.spazioit.com/software/naviserver-4.99.4-Win64.zip
>
> There are two categories of changes:
>
> 1.The ones required to have the system compiled by Visual
> Studio 2012 using as target Windows 64. They are
> identified by the #ifdef _WIN64 clause.
>
> 2.A set of necessary cosmetics/make up changes to the
> overall code base to make it more compliant with nowadays
> C STDs, and therefore more "acceptable" to nowadays C
> compilers, they are:
>
> a.I have made explicit all type conversions (with explicit
> casts)
>
> b.I have modified all functions defined in K&R C STD,
> changing them into ANSI C STD
>
> c.I have removed from the code base all reserved words,
> e.g.: new, delete, bool ...
>
> These changes make the entire codebase less "old-style"
> and more maintainable in the future.
>
> Hope it helps,
>
> Maurizio
>
>
>
> ------------------------------------------------------------------------------
> Don't let slow site performance ruin your business. Deploy New Relic APM
> Deploy New Relic app performance management and know exactly
> what is happening inside your Ruby, Python, PHP, Java, and .NET app
> Try New Relic at no cost today and get our sweet Data Nerd shirt too!
> http://p.sf.net/sfu/newrelic-dev2dev
>
>
> _______________________________________________
> aolserver-talk mailing list
> aol...@li...
> https://lists.sourceforge.net/lists/listinfo/aolserver-talk
|
|
From: Jeff R. <dv...@di...> - 2012-10-17 05:26:01
|
Hi Maurizio, I appreciate your efforts on the windows portability and build front; but on the issue of c++ acceptability, I think you are trying to do something that really doesn't need doing. naviserver/aolserver is written in C. C is not C++. C++ is not "new C" or "a better C" - it is C++. Making the code look a little more palatable to programmers who know C++ but not C is asking for more trouble. I would expect an enthusiastic young C++ programmer to see all those casts from ns_malloc and ask "why is this using old-fashioned casts and malloc, when it should be using type-safe new?" And the answer is that C is not C++; in ansi C casting from a void * to another data pointer type is explicitly a safe operation. The cast is unnecessary noise. Many C++ programmers will tell you that casts are a sign of poor design. In C++ that may be true. C is not C++. Similarly, aolserver/naviserver is not hello.c. I don't mean to turn away new programmers - on the contrary, I think the code is very clean and understandable, but it is still complicated; if a programmer is going to get confused by seeing "new" as a variable name instead of a reserved word then they're going to be utterly baffled trying to understand race conditions in rapid thread creation or the lifecycle of conn threads. So what this really amounts to is a suggestion to change the coding standards and build/warning flags for the project. That is a valid topic of discussion. I personally don't think they need changing, but I speak for me and not for anyone else involved, and as I'm sure you're well aware coding standards are things that holy wars are made of so there's likely to be no shortage of opinion on it. If you wish to change the coding standards, it should be brought up as such, and not under the guise of windows portability, which it has nothing to do with. -J PS: re: tclxkeylist.c - ok, perhaps not so surprising there. I suspect that code was imported verbatim around 1998 and updated very little since then. I wonder how much this functionality is still used, or if it would make sense to move it into a separate loadable module (could users just use a complete tclx, or would that interact badly with signal handling or other bits?), since I would expect most new code to use dict instead. Maurizio Martignano wrote: > Dear Jeff, > Thank you very much for your remarks. I would like to share what I > think. > > 2.a > Of course we are talking about C files and not C++ files, so any compiler > will accept the sources as they are (at least till now). But the point here > I believe is making the codebase easy to read and maintain. Nowadays > programmers (unlike us I am afraid - I am 52 years old) are used to C++ > more than C; so trying to use the common subset between ISO C e C++ standard > make sense. If I take gcc and I compile the code with the option > "-Wc++-compat" I get a warning for every ISO C construct that is outside of > the common subset of ISO C and ISO C++, e.g. request for implicit conversion > from void * to a pointer to non-void type. > So that is an effort in making the code easily readable by programmers (not > compilers) with C++ background. > > 2.b > Shocking? Have a look at the file "tclXkeylist.c"... The same construct is > used here and there in other places (look at the diffs). > > 2.c > Well the new STD 201X defines as keyword (see section 6.4.1) "_Bool", but > there's a macro definition converting "bool" to "_Bool" . For "new" and > "delete" the point is once again avoiding mismatches between C and C++, > causing confusion to a reader/programmer/maintainer with a C++ mindset. > > Summing up: > 1. I have done a static code analysis of all the codebase. > 2. I have identified these discrepancies and corrected all of them > 3. This makes the overall system easier to read and to maintain > > BTW: 90% of my current business (bread and butter) comes from doing this > type of analysis on various systems. Most of the time it is for embedded > systems used in avionics applications. There the language is mostly Ada and > not C or C++. > > Hope it clarifies my point, > Maurizio > > > -----Original Message----- > From: Jeff Rogers [mailto:dv...@di...] > Sent: 16 October 2012 01:38 > To: Maurizio Martignano > Cc: aol...@li... > Subject: Re: [AOLSERVER] Naviserver Win-64 Sources > > Maurizio Martignano wrote: > >> 2.A set of necessary cosmetics/make up changes to the overall code >> base to make it more compliant with nowadays C STDs, and therefore >> more "acceptable" to nowadays C compilers, they are: >> >> a.I have made explicit all type conversions (with explicit casts) >> >> b.I have modified all functions defined in K&R C STD, changing them >> into ANSI C STD >> >> c.I have removed from the code base all reserved words, e.g.: new, >> delete, bool . > > Are you building with a C compiler or a c++ compiler? new, delete, and bool > are not reserved words in any dialect of C that I'm aware of. > C++ is also pickier about casting; in C you should be able to cast any > pointer to or from a void* without a cast. > > The K&R definitions can go; It's somewhat shocking there would be any > still around. > > -J > |
|
From: Zoran V. <zv...@ar...> - 2012-10-17 08:12:32
|
On 17.10.2012, at 07:25, Jeff Rogers wrote: > (could > users just use a complete tclx, or would that interact badly with signal > handling or other bits?) It would. |
|
From: Jeff R. <dv...@di...> - 2012-10-17 08:57:43
|
Maurizio Martignano wrote: > OK > > Simple reason: Visual Studio complains about that stuff and it is annoying. > I changed that everywhere. It doesn't do any arm, and for you it is free. > Why you just do not accept it? If the problem is that VS is emitting a meaningless warning, I would first check to see if there's a way to disable that particular warning. The code change does do harm - it adds in noise - and it's not free - the stylistic change must be maintained on all future code changes. As I said before tho, I speak for myself only, and a stylistic change like this should be agreed upon (or discounted) by the community. > And the same goes for the keywords "new" and "delete". On the contrary the > keyword "bool" as I already mentioned interferes with macro definition on > "bool" in the C201X STD. > > The K&R STD function style was not only in the file I mentioned, but as I > mentioned is present also one or two more places in the codebase, so feel > free to check my archive and do a diff. I didn't review all your code changes, I'm just commenting on what you said you changed. I agreed with you on the K&R style definitions. That is a far less controversial change. > If all this C/C++ compatibility is so "crazy" and "non-sense" how come > there's an option in gcc checking for a common subset? I don't recall calling it crazy or nonsense. I said I think it's not a good idea. > I do not know if you are aware of the MISRA-C rules[...] I'm not familiar with them beyond a few minutes of web searching. But from those few minutes it's pretty clear that as/ns could not possibly conform to those rules, at least the 1998 version. (break, continue, and function pointers are deal-breakers) But those rules appear to be targeted at embedded and/or realtime systems; tho we might like as/ns to be realtime, it isn't :) > Anyhow. All of this in not really important. I do not want to be annoyed by > Visual Studio. I will use my version of the codebase. Want to use it? Please > do, you are very welcome. Want to keep the original version, it is ok too. You asked for comments, and I offered mine. I expect others will chime in too, and they may well agree with you. Either way, we can still find some way to work together. -J > > -----Original Message----- > From: Jeff Rogers [mailto:dv...@di...] > Sent: 17 October 2012 07:26 > To: Maurizio Martignano > Cc: nav...@li... > Subject: Re: [AOLSERVER] Naviserver Win-64 Sources > > Hi Maurizio, > > I appreciate your efforts on the windows portability and build front; but on > the issue of c++ acceptability, I think you are trying to do something that > really doesn't need doing. > > naviserver/aolserver is written in C. C is not C++. C++ is not "new C" > or "a better C" - it is C++. Making the code look a little more palatable > to programmers who know C++ but not C is asking for more trouble. I would > expect an enthusiastic young C++ programmer to see all those casts from > ns_malloc and ask "why is this using old-fashioned > casts and malloc, when it should be using type-safe new?" And the > answer is that C is not C++; in ansi C casting from a void * to another data > pointer type is explicitly a safe operation. The cast is unnecessary noise. > Many C++ programmers will tell you that casts are a sign of poor design. In > C++ that may be true. C is not C++. > > Similarly, aolserver/naviserver is not hello.c. I don't mean to turn away > new programmers - on the contrary, I think the code is very clean and > understandable, but it is still complicated; if a programmer is going to > get confused by seeing "new" as a variable name instead of a reserved word > then they're going to be utterly baffled trying to understand race > conditions in rapid thread creation or the lifecycle of conn threads. > > So what this really amounts to is a suggestion to change the coding > standards and build/warning flags for the project. That is a valid topic of > discussion. I personally don't think they need changing, but I speak for me > and not for anyone else involved, and as I'm sure you're well aware coding > standards are things that holy wars are made of so there's likely to be no > shortage of opinion on it. If you wish to change the coding standards, it > should be brought up as such, and not under the guise of windows > portability, which it has nothing to do with. > > -J > > PS: re: tclxkeylist.c - ok, perhaps not so surprising there. I suspect that > code was imported verbatim around 1998 and updated very little since then. > I wonder how much this functionality is still used, or if it would make > sense to move it into a separate loadable module (could users just use a > complete tclx, or would that interact badly with signal handling or other > bits?), since I would expect most new code to use dict instead. > > > Maurizio Martignano wrote: >> Dear Jeff, >> Thank you very much for your remarks. I would like to share what I >> think. >> >> 2.a >> Of course we are talking about C files and not C++ files, so any >> compiler will accept the sources as they are (at least till now). But >> the point here I believe is making the codebase easy to read and >> maintain. Nowadays programmers (unlike us I am afraid - I am 52 years >> old) are used to C++ more than C; so trying to use the common subset >> between ISO C e C++ standard make sense. If I take gcc and I compile >> the code with the option "-Wc++-compat" I get a warning for every ISO >> C construct that is outside of the common subset of ISO C and ISO C++, >> e.g. request for implicit conversion from void * to a pointer to non-void > type. >> So that is an effort in making the code easily readable by programmers >> (not >> compilers) with C++ background. >> >> 2.b >> Shocking? Have a look at the file "tclXkeylist.c"... The same >> construct is used here and there in other places (look at the diffs). >> >> 2.c >> Well the new STD 201X defines as keyword (see section 6.4.1) "_Bool", >> but there's a macro definition converting "bool" to "_Bool" . For >> "new" and "delete" the point is once again avoiding mismatches between >> C and C++, causing confusion to a reader/programmer/maintainer with a C++ > mindset. >> >> Summing up: >> 1. I have done a static code analysis of all the codebase. >> 2. I have identified these discrepancies and corrected all of them 3. >> This makes the overall system easier to read and to maintain >> >> BTW: 90% of my current business (bread and butter) comes from doing >> this type of analysis on various systems. Most of the time it is for >> embedded systems used in avionics applications. There the language is >> mostly Ada and not C or C++. >> >> Hope it clarifies my point, >> Maurizio >> >> >> -----Original Message----- >> From: Jeff Rogers [mailto:dv...@di...] >> Sent: 16 October 2012 01:38 >> To: Maurizio Martignano >> Cc: aol...@li... >> Subject: Re: [AOLSERVER] Naviserver Win-64 Sources >> >> Maurizio Martignano wrote: >> >>> 2.A set of necessary cosmetics/make up changes to the overall code >>> base to make it more compliant with nowadays C STDs, and therefore >>> more "acceptable" to nowadays C compilers, they are: >>> >>> a.I have made explicit all type conversions (with explicit casts) >>> >>> b.I have modified all functions defined in K&R C STD, changing them >>> into ANSI C STD >>> >>> c.I have removed from the code base all reserved words, e.g.: new, >>> delete, bool . >> >> Are you building with a C compiler or a c++ compiler? new, delete, >> and bool are not reserved words in any dialect of C that I'm aware of. >> C++ is also pickier about casting; in C you should be able to cast any >> pointer to or from a void* without a cast. >> >> The K&R definitions can go; It's somewhat shocking there would be any >> still around. >> >> -J >> |
|
From: Gustaf N. <ne...@wu...> - 2012-10-17 09:47:10
|
Maurizio,
the changes you have done are textually quite large, the
patch is 6000 lines +.
Please, don't expect, we can apply this and forget about it,
we need some time
to digest this. Please, don't be impatient, we all have
limited time.
For some problems, it seems for me to be better to follow the
tcl conventions, since it is not unlikely, that who ever
works on the details of
naviserver will have to deal with the tcl sources as well.
What it makes this also more time consuming is that some of
the changes should
be probably done differently, like the cases described below
that i have picked
randomly. This list is most like not complete.
all the best
-gustaf neumann
+#ifdef _WIN64
+ Ns_DStringPrintf(dsPtr, "%d %d %d %l64d %l64d %l64d
%l64d",
+#else
Ns_DStringPrintf(dsPtr, "%d %d %d %" PRId64 " %"
PRId64 " %" PRId64 " %" PRId64,
+#endif
question arise: is this just _WIN64 or as well _WIN32?
Why not define PRId64 correctly for _WIN64? (and _WIN32?)
Or, why have you commented out certain code (a) for win64
and (b) for all platforms
which is certain useful:
#ifdef NS_NOCOMPAT
-# error "No compatibility macros at present"
+// # error "No compatibility macros at present"
#endif
...
#ifdef HAVE_TCL_GETMEMORYINFO
- Tcl_GetMemoryInfo(&ds);
- Tcl_DStringResult(interp, &ds);
+ // Tcl_GetMemoryInfo(&ds);
+ // Tcl_DStringResult(interp, &ds);
#endif
return TCL_OK;
another problem is that there are some fixes included, which
have nothing to do with
_WIN64 and which are fixed already in the tip version on
bitbucket.
--- /usr/local/src/naviserver-4.99.4/nsd/compress.c
2010-03-07 17:08:38.000000000 +0100
+++ ./nsd/compress.c 2012-10-13 10:03:54.000000000 +0200
@@ -299,7 +299,11 @@
return;
}
+#ifdef _WIN64
+int
+#else
char *
+#endif
Ns_CompressBufsGzip(Ns_CompressStream *stream, struct
iovec *bufs, int nbufs, int flags,
Ns_DString *dsPtr)
|
|
From: Maurizio M. <Mau...@sp...> - 2012-10-17 10:35:25
|
Dear Gustav,
Thank you for your mail message.
My answer here below.
Maurizio
-----Original Message-----
From: Gustaf Neumann [mailto:ne...@wu...]
Sent: 17 October 2012 11:47
To: nav...@li...
Subject: Re: [naviserver-devel] [AOLSERVER] Naviserver Win-64 Sources
Maurizio,
the changes you have done are textually quite large, the patch is 6000 lines
+.
Please, don't expect, we can apply this and forget about it, we need some
time to digest this. Please, don't be impatient, we all have limited time.
For some problems, it seems for me to be better to follow the tcl
conventions, since it is not unlikely, that who ever works on the details of
naviserver will have to deal with the tcl sources as well.
What it makes this also more time consuming is that some of the changes
should be probably done differently, like the cases described below that i
have picked randomly. This list is most like not complete.
all the best
-gustaf neumann
+#ifdef _WIN64
+ Ns_DStringPrintf(dsPtr, "%d %d %d %l64d %l64d %l64d
%l64d",
+#else
Ns_DStringPrintf(dsPtr, "%d %d %d %" PRId64 " %"
PRId64 " %" PRId64 " %" PRId64,
+#endif
question arise: is this just _WIN64 or as well _WIN32?
Why not define PRId64 correctly for _WIN64? (and _WIN32?)
[MM] Because the PRId64 thing works with gcc and not with Visual Studio (or
to be correct I did not manage to make it work). I believe it works if you
use MinGW, so this is why I used only _WIN64
Or, why have you commented out certain code (a) for win64 and (b) for all
platforms which is certain useful:
#ifdef NS_NOCOMPAT
-# error "No compatibility macros at present"
+// # error "No compatibility macros at present"
#endif
This is my error, sorry, should be under _WIN64
...
#ifdef HAVE_TCL_GETMEMORYINFO
- Tcl_GetMemoryInfo(&ds);
- Tcl_DStringResult(interp, &ds);
+ // Tcl_GetMemoryInfo(&ds);
+ // Tcl_DStringResult(interp, &ds);
#endif
return TCL_OK;
same as above under _WIN64
another problem is that there are some fixes included, which have nothing to
do with
_WIN64 and which are fixed already in the tip version on bitbucket.
--- /usr/local/src/naviserver-4.99.4/nsd/compress.c
2010-03-07 17:08:38.000000000 +0100
+++ ./nsd/compress.c 2012-10-13 10:03:54.000000000 +0200
@@ -299,7 +299,11 @@
return;
}
+#ifdef _WIN64
+int
+#else
char *
+#endif
Ns_CompressBufsGzip(Ns_CompressStream *stream, struct iovec *bufs, int
nbufs, int flags,
Ns_DString *dsPtr)
[MM]
That looked to me as an error, but did not dare to correct it. Just correct
the fix and take away the _WIN64 stuff if you believe it is right
----------------------------------------------------------------------------
--
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics Download AppDynamics Lite for
free today:
http://p.sf.net/sfu/appdyn_sfd2d_oct
_______________________________________________
naviserver-devel mailing list
nav...@li...
https://lists.sourceforge.net/lists/listinfo/naviserver-devel
|
|
From: Gustaf N. <ne...@wu...> - 2012-10-17 12:06:03
|
On 17.10.12 12:35, Maurizio Martignano wrote: > > +#ifdef _WIN64 > + Ns_DStringPrintf(dsPtr, "%d %d %d %l64d %l64d %l64d > %l64d", > +#else > Ns_DStringPrintf(dsPtr, "%d %d %d %" PRId64 " %" > PRId64 " %" PRId64 " %" PRId64, > +#endif > > > question arise: is this just _WIN64 or as well _WIN32? > Why not define PRId64 correctly for _WIN64? (and _WIN32?) > > [MM] Because the PRId64 thing works with gcc and not with Visual Studio (or > to be correct I did not manage to make it work). I believe it works if you > use MinGW, so this is why I used only _WIN64 "PRId64" has nothing to do with gcc, it is supposed to be defined by inttypes.h. From googling around it seems that these macros are often a problem with visual c, not only a problem for _WIN64. From your changes, i deduce, that PRId64 and PRIuMAX are not defined for your platform, but PRIu64 seems to be there (at least, you did not change this). Please check, if the following change fixes the issues with the PRI* macros under your platform https://bitbucket.org/naviserver/naviserver/changeset/92877ff5b9625aac023c911dcd862894cfd441ce > Or, why have you commented out certain code (a) for win64 and (b) for all > platforms which is certain useful: > > #ifdef NS_NOCOMPAT > -# error "No compatibility macros at present" > +// # error "No compatibility macros at present" > #endif > > This is my error, sorry, should be under _WIN64 > > ... Is anybody aware about NS_NOCOMPAT ? This seem to stem from the win32 project files. Can this be removed? > > #ifdef HAVE_TCL_GETMEMORYINFO > - Tcl_GetMemoryInfo(&ds); > - Tcl_DStringResult(interp, &ds); > + // Tcl_GetMemoryInfo(&ds); > + // Tcl_DStringResult(interp, &ds); > #endif > return TCL_OK; > > same as above under _WIN64 Why is this a problem with _WIN64? -gustaf neumann |
|
From: Maurizio M. <Mau...@sp...> - 2012-10-17 12:49:53
|
1.Macros are defined. 2. What is not accepted is the starting with multiple format strings, e.g. ""%d %d %d %" <- 1st string PRId64 <- 2nd string " %" <--- and so on -----Original Message----- From: Gustaf Neumann [mailto:ne...@wu...] Sent: 17 October 2012 14:06 To: nav...@li... Subject: Re: [naviserver-devel] [AOLSERVER] Naviserver Win-64 Sources On 17.10.12 12:35, Maurizio Martignano wrote: > > +#ifdef _WIN64 > + Ns_DStringPrintf(dsPtr, "%d %d %d %l64d %l64d %l64d > %l64d", > +#else > Ns_DStringPrintf(dsPtr, "%d %d %d %" PRId64 " %" > PRId64 " %" PRId64 " %" PRId64, > +#endif > > > question arise: is this just _WIN64 or as well _WIN32? > Why not define PRId64 correctly for _WIN64? (and _WIN32?) > > [MM] Because the PRId64 thing works with gcc and not with Visual > Studio (or to be correct I did not manage to make it work). I believe > it works if you use MinGW, so this is why I used only _WIN64 "PRId64" has nothing to do with gcc, it is supposed to be defined by inttypes.h. From googling around it seems that these macros are often a problem with visual c, not only a problem for _WIN64. From your changes, i deduce, that PRId64 and PRIuMAX are not defined for your platform, but PRIu64 seems to be there (at least, you did not change this). Please check, if the following change fixes the issues with the PRI* macros under your platform https://bitbucket.org/naviserver/naviserver/changeset/92877ff5b9625aac023c91 1dcd862894cfd441ce > Or, why have you commented out certain code (a) for win64 and (b) for > all platforms which is certain useful: > > #ifdef NS_NOCOMPAT > -# error "No compatibility macros at present" > +// # error "No compatibility macros at present" > #endif > > This is my error, sorry, should be under _WIN64 > > ... Is anybody aware about NS_NOCOMPAT ? This seem to stem from the win32 project files. Can this be removed? > > #ifdef HAVE_TCL_GETMEMORYINFO > - Tcl_GetMemoryInfo(&ds); > - Tcl_DStringResult(interp, &ds); > + // Tcl_GetMemoryInfo(&ds); > + // Tcl_DStringResult(interp, &ds); > #endif > return TCL_OK; > > same as above under _WIN64 Why is this a problem with _WIN64? -gustaf neumann ---------------------------------------------------------------------------- -- Everyone hates slow websites. So do we. Make your web apps faster with AppDynamics Download AppDynamics Lite for free today: http://p.sf.net/sfu/appdyn_sfd2d_oct _______________________________________________ naviserver-devel mailing list nav...@li... https://lists.sourceforge.net/lists/listinfo/naviserver-devel |
|
From: Gustaf N. <ne...@wu...> - 2012-10-17 13:03:26
|
On 17.10.12 14:49, Maurizio Martignano wrote: > 1.Macros are defined. > 2. What is not accepted is the starting with multiple format strings, e.g. > ""%d %d %d %" <- 1st string PRId64 <- 2nd string " %" <--- and so on can you rephrase this. What do you mean with "starting with multiple format strings"? With the macros defined, the string "%d %d %d %" PRId64 " %" PRId64 " %" PRId64 " %" PRId64 is equivalent with "%d %d %d %" "I64d" " %" "I64d" " %" "I64d" " %" "I64d" is equivalent with "%d %d %d %I64d %I64d %I64d %I64d" -gn > > -----Original Message----- > From: Gustaf Neumann [mailto:ne...@wu...] > Sent: 17 October 2012 14:06 > To: nav...@li... > Subject: Re: [naviserver-devel] [AOLSERVER] Naviserver Win-64 Sources > > On 17.10.12 12:35, Maurizio Martignano wrote: >> +#ifdef _WIN64 >> + Ns_DStringPrintf(dsPtr, "%d %d %d %l64d %l64d %l64d >> %l64d", >> +#else >> Ns_DStringPrintf(dsPtr, "%d %d %d %" PRId64 " %" >> PRId64 " %" PRId64 " %" PRId64, >> +#endif >> >> >> question arise: is this just _WIN64 or as well _WIN32? >> Why not define PRId64 correctly for _WIN64? (and _WIN32?) >> >> [MM] Because the PRId64 thing works with gcc and not with Visual >> Studio (or to be correct I did not manage to make it work). I believe >> it works if you use MinGW, so this is why I used only _WIN64 > "PRId64" has nothing to do with gcc, it is supposed to be defined by > inttypes.h. From googling around it seems that these macros are often a > problem with visual c, not only a problem for _WIN64. > > From your changes, i deduce, that PRId64 and PRIuMAX are not defined for > your platform, but PRIu64 seems to be there (at least, you did not change > this). Please check, if the following change fixes the issues with the PRI* > macros under your platform > https://bitbucket.org/naviserver/naviserver/changeset/92877ff5b9625aac023c91 > 1dcd862894cfd441ce > >> Or, why have you commented out certain code (a) for win64 and (b) for >> all platforms which is certain useful: >> >> #ifdef NS_NOCOMPAT >> -# error "No compatibility macros at present" >> +// # error "No compatibility macros at present" >> #endif >> >> This is my error, sorry, should be under _WIN64 >> >> ... > Is anybody aware about NS_NOCOMPAT ? > This seem to stem from the win32 project files. > Can this be removed? > >> #ifdef HAVE_TCL_GETMEMORYINFO >> - Tcl_GetMemoryInfo(&ds); >> - Tcl_DStringResult(interp, &ds); >> + // Tcl_GetMemoryInfo(&ds); >> + // Tcl_DStringResult(interp, &ds); >> #endif >> return TCL_OK; >> >> same as above under _WIN64 > Why is this a problem with _WIN64? > > -gustaf neumann > > ---------------------------------------------------------------------------- > -- > Everyone hates slow websites. So do we. > Make your web apps faster with AppDynamics Download AppDynamics Lite for > free today: > http://p.sf.net/sfu/appdyn_sfd2d_oct > _______________________________________________ > naviserver-devel mailing list > nav...@li... > https://lists.sourceforge.net/lists/listinfo/naviserver-devel > > > ------------------------------------------------------------------------------ > Everyone hates slow websites. So do we. > Make your web apps faster with AppDynamics > Download AppDynamics Lite for free today: > http://p.sf.net/sfu/appdyn_sfd2d_oct > _______________________________________________ > naviserver-devel mailing list > nav...@li... > https://lists.sourceforge.net/lists/listinfo/naviserver-devel |
|
From: Maurizio M. <Mau...@sp...> - 2012-10-17 13:11:52
|
No, sorry, wrong and quick answer. Ibrahim observation stays valid. Apologies, Maurizio -----Original Message----- From: Gustaf Neumann [mailto:ne...@wu...] Sent: 17 October 2012 15:03 To: nav...@li... Subject: Re: [naviserver-devel] [AOLSERVER] Naviserver Win-64 Sources On 17.10.12 14:49, Maurizio Martignano wrote: > 1.Macros are defined. > 2. What is not accepted is the starting with multiple format strings, e.g. > ""%d %d %d %" <- 1st string PRId64 <- 2nd string " %" <--- and so on can you rephrase this. What do you mean with "starting with multiple format strings"? With the macros defined, the string "%d %d %d %" PRId64 " %" PRId64 " %" PRId64 " %" PRId64 is equivalent with "%d %d %d %" "I64d" " %" "I64d" " %" "I64d" " %" "I64d" is equivalent with "%d %d %d %I64d %I64d %I64d %I64d" -gn > > -----Original Message----- > From: Gustaf Neumann [mailto:ne...@wu...] > Sent: 17 October 2012 14:06 > To: nav...@li... > Subject: Re: [naviserver-devel] [AOLSERVER] Naviserver Win-64 Sources > > On 17.10.12 12:35, Maurizio Martignano wrote: >> +#ifdef _WIN64 >> + Ns_DStringPrintf(dsPtr, "%d %d %d %l64d %l64d %l64d >> %l64d", >> +#else >> Ns_DStringPrintf(dsPtr, "%d %d %d %" PRId64 " %" >> PRId64 " %" PRId64 " %" PRId64, >> +#endif >> >> >> question arise: is this just _WIN64 or as well _WIN32? >> Why not define PRId64 correctly for _WIN64? (and _WIN32?) >> >> [MM] Because the PRId64 thing works with gcc and not with Visual >> Studio (or to be correct I did not manage to make it work). I believe >> it works if you use MinGW, so this is why I used only _WIN64 > "PRId64" has nothing to do with gcc, it is supposed to be defined by > inttypes.h. From googling around it seems that these macros are often > a problem with visual c, not only a problem for _WIN64. > > From your changes, i deduce, that PRId64 and PRIuMAX are not defined > for your platform, but PRIu64 seems to be there (at least, you did not > change this). Please check, if the following change fixes the issues > with the PRI* macros under your platform > https://bitbucket.org/naviserver/naviserver/changeset/92877ff5b9625aac > 023c91 > 1dcd862894cfd441ce > >> Or, why have you commented out certain code (a) for win64 and (b) for >> all platforms which is certain useful: >> >> #ifdef NS_NOCOMPAT >> -# error "No compatibility macros at present" >> +// # error "No compatibility macros at present" >> #endif >> >> This is my error, sorry, should be under _WIN64 >> >> ... > Is anybody aware about NS_NOCOMPAT ? > This seem to stem from the win32 project files. > Can this be removed? > >> #ifdef HAVE_TCL_GETMEMORYINFO >> - Tcl_GetMemoryInfo(&ds); >> - Tcl_DStringResult(interp, &ds); >> + // Tcl_GetMemoryInfo(&ds); >> + // Tcl_DStringResult(interp, &ds); >> #endif >> return TCL_OK; >> >> same as above under _WIN64 > Why is this a problem with _WIN64? > > -gustaf neumann > > ---------------------------------------------------------------------- > ------ > -- > Everyone hates slow websites. So do we. > Make your web apps faster with AppDynamics Download AppDynamics Lite > for free today: > http://p.sf.net/sfu/appdyn_sfd2d_oct > _______________________________________________ > naviserver-devel mailing list > nav...@li... > https://lists.sourceforge.net/lists/listinfo/naviserver-devel > > > ---------------------------------------------------------------------- > -------- Everyone hates slow websites. So do we. > Make your web apps faster with AppDynamics Download AppDynamics Lite > for free today: > http://p.sf.net/sfu/appdyn_sfd2d_oct > _______________________________________________ > naviserver-devel mailing list > nav...@li... > https://lists.sourceforge.net/lists/listinfo/naviserver-devel ---------------------------------------------------------------------------- -- Everyone hates slow websites. So do we. Make your web apps faster with AppDynamics Download AppDynamics Lite for free today: http://p.sf.net/sfu/appdyn_sfd2d_oct _______________________________________________ naviserver-devel mailing list nav...@li... https://lists.sourceforge.net/lists/listinfo/naviserver-devel |
|
From: Gustaf N. <ne...@wu...> - 2012-10-19 07:03:46
|
On 17.10.12 14:05, Gustaf Neumann wrote: >> #ifdef NS_NOCOMPAT >> -# error "No compatibility macros at present" >> +// # error "No compatibility macros at present" >> #endif >> >> This is my error, sorry, should be under _WIN64 >> >> ... > Is anybody aware about NS_NOCOMPAT ? > This seem to stem from the win32 project files. > Can this be removed? since this is apparently of no use anymore (maurizio could compile the code), i've removed this macro. -gustaf neumann |
|
From: Ibrahim T. <it...@ar...> - 2012-10-17 12:25:43
|
Hi Gustaf, Quickly grepped all include files. No PRI* defined anywhere. Maurizio must have missed this one. Cheers, Ibrahim On 17-Oct-12 14:05, Gustaf Neumann wrote: > On 17.10.12 12:35, Maurizio Martignano wrote: > From your changes, i deduce, that PRId64 and PRIuMAX are > not defined for your platform, but PRIu64 seems to be there > (at least, you did not change this). Please check, if the > following > change fixes the issues with the PRI* macros under your platform > https://bitbucket.org/naviserver/naviserver/changeset/92877ff5b9625aac023c911dcd862894cfd441ce |
|
From: Gustaf N. <ne...@wu...> - 2012-10-17 12:55:36
|
Is this maybe version dependent? to be on the safe side, i added PRIu64 in case it is not defined -g On 17.10.12 14:25, Ibrahim Tannir wrote: > Hi Gustaf, > > Quickly grepped all include files. No PRI* defined anywhere. > Maurizio must have missed this one. > > Cheers, > Ibrahim > > On 17-Oct-12 14:05, Gustaf Neumann wrote: >> On 17.10.12 12:35, Maurizio Martignano wrote: >> From your changes, i deduce, that PRId64 and PRIuMAX are >> not defined for your platform, but PRIu64 seems to be there >> (at least, you did not change this). Please check, if the >> following >> change fixes the issues with the PRI* macros under your platform >> https://bitbucket.org/naviserver/naviserver/changeset/92877ff5b9625aac023c911dcd862894cfd441ce > > ------------------------------------------------------------------------------ > Everyone hates slow websites. So do we. > Make your web apps faster with AppDynamics > Download AppDynamics Lite for free today: > http://p.sf.net/sfu/appdyn_sfd2d_oct > _______________________________________________ > naviserver-devel mailing list > nav...@li... > https://lists.sourceforge.net/lists/listinfo/naviserver-devel |
|
From: Maurizio M. <Mau...@sp...> - 2012-10-17 08:55:53
|
OK Simple reason: Visual Studio complains about that stuff and it is annoying. I changed that everywhere. It doesn't do any arm, and for you it is free. Why you just do not accept it? And the same goes for the keywords "new" and "delete". On the contrary the keyword "bool" as I already mentioned interferes with macro definition on "bool" in the C201X STD. The K&R STD function style was not only in the file I mentioned, but as I mentioned is present also one or two more places in the codebase, so feel free to check my archive and do a diff. If all this C/C++ compatibility is so "crazy" and "non-sense" how come there's an option in gcc checking for a common subset? I do not know if you are aware of the MISRA-C rules (Guidelines for the use of C language in critical systems - http://en.wikipedia.org/wiki/MISRA_C); I do cause I work on this stuff - my bread and butter. Well in the 1998 version of this standard there was this rule: Rule 44 - Redundant explicit casts should be not used. But in the 2004 version of the standard this rule has been "rescinded", not by me, but by the authors of the standard, because for some people it was controversial and for some others it did not make any sense. Anyhow. All of this in not really important. I do not want to be annoyed by Visual Studio. I will use my version of the codebase. Want to use it? Please do, you are very welcome. Want to keep the original version, it is ok too. Hope it helps, Maurizio -----Original Message----- From: Jeff Rogers [mailto:dv...@di...] Sent: 17 October 2012 07:26 To: Maurizio Martignano Cc: nav...@li... Subject: Re: [AOLSERVER] Naviserver Win-64 Sources Hi Maurizio, I appreciate your efforts on the windows portability and build front; but on the issue of c++ acceptability, I think you are trying to do something that really doesn't need doing. naviserver/aolserver is written in C. C is not C++. C++ is not "new C" or "a better C" - it is C++. Making the code look a little more palatable to programmers who know C++ but not C is asking for more trouble. I would expect an enthusiastic young C++ programmer to see all those casts from ns_malloc and ask "why is this using old-fashioned casts and malloc, when it should be using type-safe new?" And the answer is that C is not C++; in ansi C casting from a void * to another data pointer type is explicitly a safe operation. The cast is unnecessary noise. Many C++ programmers will tell you that casts are a sign of poor design. In C++ that may be true. C is not C++. Similarly, aolserver/naviserver is not hello.c. I don't mean to turn away new programmers - on the contrary, I think the code is very clean and understandable, but it is still complicated; if a programmer is going to get confused by seeing "new" as a variable name instead of a reserved word then they're going to be utterly baffled trying to understand race conditions in rapid thread creation or the lifecycle of conn threads. So what this really amounts to is a suggestion to change the coding standards and build/warning flags for the project. That is a valid topic of discussion. I personally don't think they need changing, but I speak for me and not for anyone else involved, and as I'm sure you're well aware coding standards are things that holy wars are made of so there's likely to be no shortage of opinion on it. If you wish to change the coding standards, it should be brought up as such, and not under the guise of windows portability, which it has nothing to do with. -J PS: re: tclxkeylist.c - ok, perhaps not so surprising there. I suspect that code was imported verbatim around 1998 and updated very little since then. I wonder how much this functionality is still used, or if it would make sense to move it into a separate loadable module (could users just use a complete tclx, or would that interact badly with signal handling or other bits?), since I would expect most new code to use dict instead. Maurizio Martignano wrote: > Dear Jeff, > Thank you very much for your remarks. I would like to share what I > think. > > 2.a > Of course we are talking about C files and not C++ files, so any > compiler will accept the sources as they are (at least till now). But > the point here I believe is making the codebase easy to read and > maintain. Nowadays programmers (unlike us I am afraid - I am 52 years > old) are used to C++ more than C; so trying to use the common subset > between ISO C e C++ standard make sense. If I take gcc and I compile > the code with the option "-Wc++-compat" I get a warning for every ISO > C construct that is outside of the common subset of ISO C and ISO C++, > e.g. request for implicit conversion from void * to a pointer to non-void type. > So that is an effort in making the code easily readable by programmers > (not > compilers) with C++ background. > > 2.b > Shocking? Have a look at the file "tclXkeylist.c"... The same > construct is used here and there in other places (look at the diffs). > > 2.c > Well the new STD 201X defines as keyword (see section 6.4.1) "_Bool", > but there's a macro definition converting "bool" to "_Bool" . For > "new" and "delete" the point is once again avoiding mismatches between > C and C++, causing confusion to a reader/programmer/maintainer with a C++ mindset. > > Summing up: > 1. I have done a static code analysis of all the codebase. > 2. I have identified these discrepancies and corrected all of them 3. > This makes the overall system easier to read and to maintain > > BTW: 90% of my current business (bread and butter) comes from doing > this type of analysis on various systems. Most of the time it is for > embedded systems used in avionics applications. There the language is > mostly Ada and not C or C++. > > Hope it clarifies my point, > Maurizio > > > -----Original Message----- > From: Jeff Rogers [mailto:dv...@di...] > Sent: 16 October 2012 01:38 > To: Maurizio Martignano > Cc: aol...@li... > Subject: Re: [AOLSERVER] Naviserver Win-64 Sources > > Maurizio Martignano wrote: > >> 2.A set of necessary cosmetics/make up changes to the overall code >> base to make it more compliant with nowadays C STDs, and therefore >> more "acceptable" to nowadays C compilers, they are: >> >> a.I have made explicit all type conversions (with explicit casts) >> >> b.I have modified all functions defined in K&R C STD, changing them >> into ANSI C STD >> >> c.I have removed from the code base all reserved words, e.g.: new, >> delete, bool . > > Are you building with a C compiler or a c++ compiler? new, delete, > and bool are not reserved words in any dialect of C that I'm aware of. > C++ is also pickier about casting; in C you should be able to cast any > pointer to or from a void* without a cast. > > The K&R definitions can go; It's somewhat shocking there would be any > still around. > > -J > |
|
From: Stephen D. <sd...@gm...> - 2012-10-17 10:56:04
|
On Wed, Oct 17, 2012 at 4:55 AM, Maurizio Martignano <Mau...@sp...> wrote: > OK > > Simple reason: Visual Studio complains about that stuff and it is annoying. Looks like Visual Studio 2012 comes with a C compiler: http://msdn.microsoft.com/en-us/library/bb384838.aspx "Visual Studio includes a C compiler that you can use to create everything from basic C programs to Windows API applications." "By default, the Visual C++ compiler treats all files that end in .c as C source code," Maybe there's a bug in your build scripts that are forcing C code to be treated as C++? |
|
From: Ibrahim T. <it...@ar...> - 2012-10-16 10:10:48
|
Hi everybody,
For those who don't know me, I'm Zoran's partner and am involved
with compiling the Windows version of Tcl and Naviserver at our
company.
As Zoran mentioned in one of his posts, I was reluctant to
release our (my) changes and the VC project files, since those
are quite specific to our development and I haven't in the many
years that I have been involved yet found the time to clean them
up and make them suitable for public use. However, many of my
changes and fixes have been released through Zoran as our
representative in the community.
Just to comment Maurizio's changes related to variable names - I
suppose he has done so due to conflicts with reserved words that
MS VC++ doesn't tolerate.
To mention a couple of other problems that I have discovered over
the years and resolved rather dirtily in my compilation: The
memory and DLL model of compilation must be the same for TCL and
the Naviserver libs and must fit with the Microsoft
redistributable package. This is why I compile everything with
the same model and have made sure (through defines) that all the
packages use the TCL memory allocator. Otherwise different MS
memory allocators get involved and you get all kinds of strange
effects and memory corruptions. Furthermore, the environment
variable handling is affected. If the packages are compiled with
different models, the vars will be visible in one package, but
not in the other.
Another problem is the interchangeable use of Windows Handles and
ANSI file descriptors in the code, which does not work
consistently in Windows. We have made some adaptions here,
especially regarding the nx_proxy package in association with the
remaining nsd code. However, AFAIK, Zoran has not yet had the
time to release this yet.
Lastly, if not compiled through VC, one is not able to use the VC
debugger to debug the code. That is why I have avoided creating
make files, using the existing configure/make files and mingw.
I hope these tips may be valuable to Maurizio (and others). I
have not yet inspected his project files and tried out the
compilation, but I've seen that many have to do with casts to
silence the warning during compilation and some with avoiding
using reserved words.
I hope to find the time in the future to get more involved.
Regards,
Ibrahim
On 16-Oct-12 11:33, Gustaf Neumann wrote:
> Dear Maurizio,
>
> First of all, many thanks for your efforts! These are very welcome!
>
> I am as well somewhat surprised, that there are still pieces of
> K&R style around.
> There is really no need to keep these. i'll commit an updated
> version of tclxkeylist.c
> to mercurial. As for the other changes, i am not really convinced
> that changing
> all variables named "new" to "mm_new" helps to improve the code...
>
> for the other changes, your archive file is the full kitchen
> sink, going though
> the changes will take a while.
>
> We should move the discussion to the naviserver-devel list
>
> Best regards
> -gustaf neumann
>
>
>
>
> On 16.10.12 00:36, Maurizio Martignano wrote:
>>
>> To facilitate the discussion, I put in here some examples of
>> the changes I made:
>>
>> *2.a.*
>>
>> From
>>
>> mapPtr = ns_malloc(sizeof(Map));
>>
>> To
>>
>> mapPtr = (Map*) ns_malloc(sizeof(Map));
>>
>> *2.b.*
>>
>> From
>>
>> int
>>
>> TclX_WrongArgs (interp, commandNameObj, string)
>>
>> Tcl_Interp* interp;
>>
>> Tcl_Obj* commandNameObj;
>>
>> char * string;
>>
>> {
>>
>> To
>>
>> int
>>
>> TclX_WrongArgs (Tcl_Interp* interp, Tcl_Obj* commandNameObj,
>> char* string)
>>
>> {
>>
>> *2.c*
>>
>> From
>>
>> if(new) {
>>
>> To
>>
>> if(mm_new) {
>>
>> All these changes have been applied everywhere.
>>
>> ========
>>
>> Dear all,
>>
>> While the tests seem to go rather well but are
>> not finished yet (and it will take quite some time), I have
>> decided to make available anyhow the result of my activity.
>>
>> The sources (and Visual Studio 2012 project files) are
>> available at:
>>
>> http://www.spazioit.com/software/naviserver-4.99.4-Win64.zip
>>
>> There are two categories of changes:
>>
>> 1.The ones required to have the system compiled by Visual
>> Studio 2012 using as target Windows 64. They are identified by
>> the #ifdef _WIN64 clause.
>>
>> 2.A set of necessary cosmetics/make up changes to the overall
>> code base to make it more compliant with nowadays C STDs, and
>> therefore more “acceptable” to nowadays C compilers, they are:
>>
>> a.I have made explicit all type conversions (with explicit casts)
>>
>> b.I have modified all functions defined in K&R C STD, changing
>> them into ANSI C STD
>>
>> c.I have removed from the code base all reserved words, e.g.:
>> new, delete, bool …
>>
>> These changes make the entire codebase less “old-style” and
>> more maintainable in the future.
>>
>> Hope it helps,
>>
>> Maurizio
|
×
Want the latest updates on software, tech news, and AI?
Get latest updates about software, tech news, and AI from SourceForge directly in your inbox once a month.
Thanks for helping keep SourceForge clean.
X