consmgr-tech Mailing List for consmgr - Remote Console Manager
Status: Alpha
Brought to you by:
cross
Menu
▾
▴
consmgr-tech — Technical discussions about internal development details of the consmgr project.
You can subscribe to this list here.
| 2008 |
Jan
|
Feb
|
Mar
(1) |
Apr
(17) |
May
(2) |
Jun
|
Jul
|
Aug
|
Sep
|
Oct
|
Nov
|
Dec
|
|---|
|
From: Chris R. <cro...@di...> - 2008-05-23 00:51:47
|
On May 22, 2008, at 11:18, Chris Ross wrote:
> Geoff, you mentioned in IM yesterday that you'd been thinking you
> wanted to refactor some of the code in MasterController. I'm about to
> traipse all over the code I think you were talking about, so I'm
> wondering what you wanted to do? Could you describe it briefly?
Okay. As noted, I think this may be moot now. Take a look at my
changes. There are definately things that could be improved, and I'm
happy for criticism. There's some magic with templating that I know
(or maybe "feel"/"expect") can be done, that I don't know how to do,
but. When templating a method, it can determine the template
parameter without it needing to be given. But, that appears not to
work with classes, so one of the functor classes still needs the
specification. (detach_all, used on line 411 of common/mastercont.cc.
Ack! I realize now that I failed to svn add a new file that has
all the functors in it! Oops! I'll get that in RSN...
- Chris
|
|
From: Chris R. <cr...@di...> - 2008-05-22 15:21:57
|
Geoff, you mentioned in IM yesterday that you'd been thinking you
wanted to refactor some of the code in MasterController. I'm about to
traipse all over the code I think you were talking about, so I'm
wondering what you wanted to do? Could you describe it briefly?
Thanks.
- Chris
|
|
From: Chris R. <cr...@di...> - 2008-04-30 22:28:31
|
Okay. So, after a conversation yesterday, I decided to short-cut
the process and just repackage read data (in BiDirConn::reader) as a
message and enqueue it. So, the only thing in Messageable that reads
messages is Messageable::get_message, and nothing calls that. So,
clearly, the data never went anywhere.
We need to figure out where the hand-off should be. Either:
1) There should be a thread that watches for messages, and when
data messages come in re-queues them in some way for the writer thread
to see and write,
2) The writer needs to ask it's messageable half for messages that
are of the data type.
Either of these is doable, and while #2 seems simpler to code, it
may be the wrong answer longer term.
Input appreciated. I'm stopped at the moment.
- Chris
|
|
From: Chris R. <cr...@di...> - 2008-04-30 22:00:08
|
This is the bane of my existence right now, and making it very hard
for me to get anything working. Even though I've thrown that kludge
into Connection::manager() so that it can survive without blowing
itself to smithereens, all of the debugging statements are reporting
being part of an FDConnector, rather than the SocketConnector they
really are, because the methods are being called in the wrong order (I
think). I guess I'm not 100% sure that it's the problem in that one
case, but it's definitely a problem we need to solve.
You are the one who suggested the "Make an object do everything
it's supposed to do upon construction" methodology, which I agree has
advantages, but looks impractical given the virtual inheritance issues
we're seeing. We need to figure out some other methodology, I fear.
Thoughts?
- Chris
|
|
From: Chris R. <cro...@di...> - 2008-04-19 14:32:08
|
Okay. The work has really been done with respect to changing the
code to *use* SmartPtr from the Loki library. So, the questions that
remain (and their answers in some cases) are:
1) Required? - Do we make it required? Make a preprocessor symbol
for it like I did with each of the configuration file systems?
- Maintaining "alternate" code makes this hard enough I think we
should just require it, but it's definitely still an open question.
2) Licensing - What's the Loki code?
- It's an MIT license. Completely acceptable, and easy to use. We
just need to make sure to leave the language in their sources, and
we're good.
3) Compilation - How?
Options:
a) Build the two needed source files into libcmgr (which is what the
branch does at the moment)
b) Build a subset of loki (perhaps just those two files) into another
archive, just for clarity of code-origin
c) Build "the loki library" and link against it. The linker will
figure out that we only need those two object files, so the file size
won't change any.
4) Packaging - Which files go where in our source distribution?
- This depends a log on #3. Considerations may be that it's possible
some people have an installed loki library that we could link
against. I don't expect this, but.
Thanks. Feedback on any and all issues appreciated. I'd like to
get whatever we want folded in folded in sooner rather than later so
we can go back to working on functionality.
Geoff, any progress on the message-passing system, which you'd
indicated you thought you would work on next?
- Chris
|
|
From: <con...@li...> - 2008-04-05 15:39:37
|
On Thu, 2008-04-03 at 18:47 -0400, Geoff Adams wrote:
> You know, I had named it "tests," but then I decided that "doc" was
> singular for more than one doc, so it fit the pattern. That is, it's
> the "test" directory. The one that has tests in it. Certainly, I'm
> fine with it either way, though. :)
Yeah, I see that. I thought about doc vs. docs yesterday, too. For
some reason, "documentation" (for which I'm using the abbreviation
"doc") just works, and is plural, so it ended up being "doc". I guess
it's whether you think of doc as "document" (vs. "documents") or
"documentation". :-)
Anyway. We can ponder renaming that later if we want. Neither is
bigly important to me either. :-)
- Chris
|
|
From: Chris R. <cro...@di...> - 2008-04-03 21:19:22
|
Hey there. Okay, I'm still having a pretty big problem understanding
what's in the architecture document. Unfortuntely the "add new
information on the bottom" has left it internally inconsistent. Made
even more complicated by the class renaming.
Could you please coalasce the section that starts "Listener Redesign"
with the one that starts "One More Step[...]" ? The information in the
former of those two sections is I think mostly unimportant, if not
actually incorrect in places, given the later redesign described in the
last section.
The problem I'm having now is that I'm not sure how everything fits
together. The little descriptions of each class heirarchy and its
presumed functional responsibilities (that is under the earlier
"Listener Redesign" section) is just what I need, but I need it for the
new system where Listeners are one thing, and Connectors are another.
Particularly, the information about Connectors up in that earlier
section still suggest that a Connector (subclass) should have
information and functionality about incoming connections, but I believe
this is no longer true, correct?
Does Connector still have information about how to establish a
connection, *and* information about how to read and write on the
connection after it is established? So, we need a "are we connected"
boolean (which I think is already there, pulled from
Connection/Outlet/BiDirConn)
Thanks. I think that bit of redocumenting will help me a lot. And, if
you're going to have time to work on the code, I'm happy to just let you
do that to finish that refactoring. Things are a mess right now, and
I'm inclined to work on it, but it's clearly a bit easier for you since
you started, and have a better idea what should be where in concept than
I do right now.
Thanks...
- Chris
|
|
From: Chris R. <cro...@di...> - 2008-04-03 12:56:21
|
On Apr 2, 2008, at 18:03, Geoff Adams wrote:
> Well, whether we said it or not, I tend to agree with that philosophy.
> (I believe we did discuss using accessors to get at things in *other*
> classes--particularly those classes that you compose into yourself as
> a way of having their functionality--which is a little different, but
> the same certainly applies to super/subclasses. So, I'm with you
> here.)
Okay, so we should be using a method to do this. Got it.
> I was in the middle of a lot of changes to these classes, and didn't
> really finish it. This defect is, in particular, probably related to
> the fact that much of this code was split out of SockConn, the Socket
> Connector (which I kinda feel like we should rename that way for
> clarity; just not the file names), which shouldn't actually know
> anything about addresses. So, I was in the middle of moving the
> address-specific stuff into SockConn's subclasses, and SocketListener
> probably inherited some of that change.
Right. I see that you were in the middle of a lot of changes,
which is why I was trying to stay out of them. There's a lot of work
yet to do, mostly (it appears) in removing code that's in the wrong
place, and/or in more than one place.
And I'd be happy for you to rename the classes. Conn was a unique
abbreviation when I used it, now that it's not, I see the value in
renaming the classes...
> Please fix this if you get the chance.
Okay. I can fix the "should use the method of superclass to reset
the superclasses data member" fix. As for the rest of the changes,
I'm not really able to do those. There are a lot of things that look
odd to me, but since I don't know why the things exist where they do,
I don't feel that I can change or modify them in code or presence.
You'll have to finish the rearchitecture of the classes, or at least
feel you're done, before I'll break into that. :-) Too messy in
there right now.
- Chris
|
|
From: Geoff A. <gsa...@av...> - 2008-04-03 05:57:20
|
On 2 Apr 2008, at 5:55 PM, Chris Ross wrote: > There's a JAVADOC_AUTOBRIEF too, which uses the javadoc style > comments, as you mention. > I think that's the /** ... */ things. I'm not sure. I didn't know > anything about javadoc, and the QT thing seemed closer to what I was > inclined to do for docs. Clearly, since you're familiar with > javadoc, I should look at that format too. doxygen totally likes > it, AFAIK. Well, really, I don't care which style we use. I'm pretty flexible that way. :) So, whatever's the most convenient, conventional with doxygen, or any such criteria is just fine with me. As long as you let me know which style we should use in the source files. - Geoff |
|
From: Chris R. <cro...@di...> - 2008-04-02 23:30:55
|
On Apr 2, 2008, at 5:38 PM, Geoff Adams wrote:
> I'm surprised that's not a default. After using JavaDoc for years, I
> generally try to make the first sentence of such comments be a simple
> summary suitable for such purposes. What you described is exactly what
> JavaDoc does. And, in fact, the JavaDoc format specifies that there
> should (must, in fact, I think) be a first sentence, including a
> period, so that it can use that for the brief description.
There's a JAVADOC_AUTOBRIEF too, which uses the javadoc style
comments, as you mention.
I think that's the /** ... */ things. I'm not sure. I didn't know
anything about javadoc, and the QT thing seemed closer to what I was
inclined to do for docs. Clearly, since you're familiar with
javadoc, I should look at that format too. doxygen totally likes
it, AFAIK.
- Chris
|
|
From: Geoff A. <ga...@av...> - 2008-04-02 22:10:05
|
On 2 Apr 2008, at 8:37 AM, Chris Ross wrote: > I know you just made a whole bunch of changes, Geoff, and that it > was a complicated commit. But, in the future, when you copy files, > please "svn copy" the files, so that they carry history. I always try to do the right thing in this respect. It's a very useful feature of Subversion, but only if you use it properly. > For example, > all of the listener subclasses which clearly take all of their code > from the old Connection subclasses have no history. In the long run > it won't matter, but it makes it harder for me looking at it now to > figure out where all of the code came from, because it's all "rev 246 > by gsadams". I wasn't thinking of these files as having come from anywhere, so I created them fresh. Certainly, though, the code came from elsewhere, so that would have made sense. Too bad I didn't think of it at the time. (The process that led to these files was not actually a simple copy, although had I known at the time what I knew by the end of the process, I would certainly have done them that way. In fact, it's still not immediately clear to me that a given new Listener subclass comes from a particular old Connection subclass, although there is probably a one-to-one correlation, simply because the hierarchy just makes sense.) > Thanks. As for listener.h, I see you deleted it and readded it. > Since it's a new thing of an old name, this I think may've been the > right (and cool) thing to do. :-) Thanks! Yes. That was the correct operation, IMHO. Subversion calls that "replace" when you do it in a single commit. :) - Geoff |
|
From: Geoff A. <gsa...@av...> - 2008-04-02 22:04:05
|
On 2 Apr 2008, at 8:34 AM, Chris Ross wrote: > Hey there. It looks like you changed UDSListener::set_addr() to > modify its inherited addr member directly, rather than call > SockConn::set_addr(). I thought we had stated earlier that we > shouldn't modify inherited members, and that there should be methods > for doing such in the class that owns the data member? SockListener > has set_addr() methods. > > Clearly, the model of coding is a little unclear somewhere. I > swear we specifically said that there should be accessor methods for > things that were members in superclasses.... Well, whether we said it or not, I tend to agree with that philosophy. (I believe we did discuss using accessors to get at things in *other* classes--particularly those classes that you compose into yourself as a way of having their functionality--which is a little different, but the same certainly applies to super/subclasses. So, I'm with you here.) I was in the middle of a lot of changes to these classes, and didn't really finish it. This defect is, in particular, probably related to the fact that much of this code was split out of SockConn, the Socket Connector (which I kinda feel like we should rename that way for clarity; just not the file names), which shouldn't actually know anything about addresses. So, I was in the middle of moving the address-specific stuff into SockConn's subclasses, and SocketListener probably inherited some of that change. Please fix this if you get the chance. Thanks, - Geoff |
|
From: Geoff A. <gsa...@av...> - 2008-04-02 21:38:57
|
On 2 Apr 2008, at 8:11 AM, Chris Ross wrote: > [ discussion about multiple lines of //! vs. /*! ... */ elided ] I see. That does all makes sense. The Doxygen format is a clear copy of the JavaDoc format, which uses /** ... */ for such things. But since Doxygen added the //! ... option, I just figured it worked the same way. But since each line beginning with // is a separate entity, I can see why it might not. However, Ruby's rdoc format is a little smarter, but it follows the ruby style, which is a little more flexible in general. Anyway, got it. > I've turned on a feature called "QT_AUTOBRIEF" in the doxygen > configuration. That will cause QT style comments (which /*! ...... */ > is, but I think //! in a group isn't. I'm not 100% sure on that, but) > to absorb the first sentence (til a period) as the "brief" comment, > and the remainder as a detailed comment. I *think* if you were to use > the first of my suggestions above, the entire block of text would be > taken as a detailed comment, and there would be no brief comment. The > brief comment is what gets displayed next to the class/file/method/etc > in the list forms, and the detail all gets put lower on the pages > where it gives a block explaining each thing [in detail]. I'm surprised that's not a default. After using JavaDoc for years, I generally try to make the first sentence of such comments be a simple summary suitable for such purposes. What you described is exactly what JavaDoc does. And, in fact, the JavaDoc format specifies that there should (must, in fact, I think) be a first sentence, including a period, so that it can use that for the brief description. - Geoff |
|
From: Geoff A. <ga...@av...> - 2008-04-02 21:24:33
|
On 2 Apr 2008, at 7:41 AM, Chris Ross wrote: > heh. It's not even conveinently editable on the Mac! I edited it > this morning, and of course it nuked the .svn directory, so I couldn't > commit (or diff) the changes. hehe. Clearly, something that is a > directory but isn't, which the Mac sometimes does, is "bad" for > portability (and subversion). I noticed that last night when I made some changes to it. What a royal PITA! Definitely not the file format to be using for this... > Oh well. Anyway, I made a few minor changes to the doc, > corrections, setting the name of MasterController, no significant > changes, but. Make sure to update before you move it to the new > location, whatever that may be. Done. It's now doc/architecture.rtf, with associated architecture-figures/*. Hopefully that'll work better. I've left it RTF, for now, because I still have fantasies about making it nicely formatted and especially easy to read. Plus, updated at some point to be more than just proposed changes, but actually the current design. :) Which I guess it's getting closer to, as we work on the source code, but it's still written as proposed changes. - Geoff |
|
From: Chris R. <cro...@di...> - 2008-04-02 12:37:16
|
I know you just made a whole bunch of changes, Geoff, and that it
was a complicated commit. But, in the future, when you copy files,
please "svn copy" the files, so that they carry history. For example,
all of the listener subclasses which clearly take all of their code
from the old Connection subclasses have no history. In the long run
it won't matter, but it makes it harder for me looking at it now to
figure out where all of the code came from, because it's all "rev 246
by gsadams".
Thanks. As for listener.h, I see you deleted it and readded it.
Since it's a new thing of an old name, this I think may've been the
right (and cool) thing to do. :-) Thanks!
- Chris
|
|
From: Chris R. <cro...@di...> - 2008-04-02 12:34:34
|
Hey there. It looks like you changed UDSListener::set_addr() to
modify its inherited addr member directly, rather than call
SockConn::set_addr(). I thought we had stated earlier that we
shouldn't modify inherited members, and that there should be methods
for doing such in the class that owns the data member? SockListener
has set_addr() methods.
Clearly, the model of coding is a little unclear somewhere. I
swear we specifically said that there should be accessor methods for
things that were members in superclasses....
Anyway...
- Chris
|
|
From: Chris R. <cro...@di...> - 2008-04-02 12:31:53
|
Okay. So, if I have time to work on this code today, is that
reasonable? Or, are you planning to be effectively in the middle of
more work?
I'm concerned about the duplication of code between the Connector
subclasses, and the Listener subclasses. For example, I had reason to
be in TCPConn and TCPListener, and they're the same code, except for
the prep_for_incoming() method existing in TCPListener. Is this just
an artifact of one having been copied from the other, or is much of
the code in both of them still needed by both of them? If so, then
something's wrong with the design, I'd argue.
Anyway. I'll discuss this with you today, I hope. I just need to
get a better understanding of where we think things are, and what work
needs to be done next.
Thanks!
- Chris
|
|
From: Chris R. <cro...@di...> - 2008-04-02 12:11:30
|
Hey there. I've seen you, once or twice, do:
//! This is a paragraph describing something. We put
// text here, and expect it to be used by doxygen. Is
// that reasonable?
This only sets the first line to doxygen. "//!" is a
one-line comment, or in a block treated as a block. So,
the above should be:
//! This is a paragraph describing something. We put
//! text here, and expect it to be used by doxygen. Is
//! that reasonable?
or, the format I've been using for block comments:
/*! This is a paragraph describing something. We put
* text here, and expect it to be used by doxygen. Is
* that reasonable?
*/
I've turned on a feature called "QT_AUTOBRIEF" in the doxygen
configuration. That will cause QT style comments (which /*! ...... */
is, but I think //! in a group isn't. I'm not 100% sure on that, but)
to absorb the first sentence (til a period) as the "brief" comment,
and the remainder as a detailed comment. I *think* if you were to use
the first of my suggestions above, the entire block of text would be
taken as a detailed comment, and there would be no brief comment. The
brief comment is what gets displayed next to the class/file/method/etc
in the list forms, and the detail all gets put lower on the pages
where it gives a block explaining each thing [in detail].
Anyway. Let me know if what I've described here makes sense. I
don't expect you to necessarily take all of this in, and I'm happy to
have that responsibility myself, but I figure you want to know the
details that matter as you go. :-)
Thanks!
- Chris
|
|
From: Chris R. <cro...@di...> - 2008-04-02 11:41:51
|
heh. It's not even conveinently editable on the Mac! I edited it
this morning, and of course it nuked the .svn directory, so I couldn't
commit (or diff) the changes. hehe. Clearly, something that is a
directory but isn't, which the Mac sometimes does, is "bad" for
portability (and subversion).
Oh well. Anyway, I made a few minor changes to the doc,
corrections, setting the name of MasterController, no significant
changes, but. Make sure to update before you move it to the new
location, whatever that may be.
Thanks! Talk to you more soon.
- Chris
|
|
From: Chris R. <cro...@di...> - 2008-03-30 01:31:11
|
Geoff, in revision 210, when you added the MasterController class,
you also for some reason made the static manage() member of Connection
private rather than public. For reasons I don't understand, things
still compile under gcc 4.0 and 4.1, but with that private, Listener
(which currently calls pthread_create on Connection::manage) fails to
compile now using gcc 4.3.
I'm not sure whether it's a bug that it compiled before or not, but.
Doesn't MasterController also need to invoke a thread on
Connection::manage? In which case, it'll need to stay public. (And,
you changed it to private, and the comment a few lines below still
discusses it being public, but that's another point.)
Thanks. Could use your thoughts on why you changed that.
- Chris
|