[Home] [Downloads] [Search] [Help/forum]

Gammon Software Solutions forum

See www.mushclient.com/spam for dealing with forum spam. Please read the MUSHclient FAQ!

[Folder]  Entire forum
-> [Folder]  MUSHclient
. -> [Folder]  Development
. . -> [Subject]  Improvements to plugin callbacks
Home  |  Users  |  Search  |  FAQ
Username:
Register forum user name
Password:
Forgotten password?
(New message)
Subject: Improvements to plugin callbacks
Name:
Your forum user name.
Register forum user name
Password:
Your forum password.
Forgotten password?
Message:
Message to be posted (in English, please).
Forum codes:
Check this if your message uses 'forum codes' or templates (auto-detected for new posts).
Forum codes Templates

Save this message ...

Subject review (reverse sequence)

Pages: 1  2 3  

Posted by Twisol   USA  (2,229 posts)  [Biography] bio
Date Mon 20 Sep 2010 06:14 AM (UTC)  [ quote  ]

Amended on Mon 20 Sep 2010 06:15 AM (UTC) by Twisol

Message
David Haley said:
Yes, I meant doing your own project, not the changes in this thread. Re: mistakes, you said that mistakes would be prevented by encapsulation and "easing out coupling".

Yes, I believe that a number of them can be prevented. It's like the age-old argument against globals, isn't it? The more you restrict access to a datum, the more you can be certain how it actually behaves. It moves the onus of good behavior from the programmer to the compiler. But I certainly don't expect these things to make mistakes or bugs impossible... That would be silly.

I guess I'll be spending more time on Aspect, at any rate. :)
'Soludra' on Achaea

Blog: http://jonathan.com/
GitHub: http://github.com/Twisol
[Go to top] top
Posted by David Haley   USA  (3,881 posts)  [Biography] bio   Moderator
Date Mon 20 Sep 2010 06:08 AM (UTC)  [ quote  ]
Message
Yes, I meant doing your own project, not the changes in this thread. Re: mistakes, you said that mistakes would be prevented by encapsulation and "easing out coupling".

I think there's lots of great work that can come out of all of this. I just don't think this particular avenue is the most productive.
David Haley aka Ksilyan
Head Programmer,
Legends of the Darkstone

http://david.the-haleys.org
[Go to top] top
Posted by Twisol   USA  (2,229 posts)  [Biography] bio
Date Mon 20 Sep 2010 06:03 AM (UTC)  [ quote  ]

Amended on Mon 20 Sep 2010 06:04 AM (UTC) by Twisol

Message
David Haley said:
yet you seem unwilling to actually go do this stuff yourself.

I'll take the other stuff, but this one seems ambiguous to me. Do you mean I'm not actually trying to do this now? The whole reason we're having this discussion is because I'm trying to do it. Do you mean actually doing my own project this way? Okay: I'll take that one too.

I think you're exaggerating my claims though. I mean, impossible to make mistakes? "magically decoupled"? I'm saying we don't have enough of these things, not that we need them 100% everywhere.

Anyways, you're right. Enough talking. At this rate I wouldn't be surprised if Nick banned me just to get some peace and quiet.
'Soludra' on Achaea

Blog: http://jonathan.com/
GitHub: http://github.com/Twisol
[Go to top] top
Posted by David Haley   USA  (3,881 posts)  [Biography] bio   Moderator
Date Mon 20 Sep 2010 05:50 AM (UTC)  [ quote  ]
Message
I'll tell you what, Twisol: you go produce a large piece of software written such that "silly mistakes" are impossible to make, producing a framework that is magically decoupled, encapsulated and orthogonal (you can add more buzzwords if you like) while actually solving some problem for end-users. My advice is not at all out of context here. I do not work on the MUSHclient source but I have worked on a good number of large projects, both ones I started and ones inherited from other peoples, enough to know that you're not focusing on the useful tasks here and are saying things that just don't make sense. :-/ You're using magic words from some theory appealing to some aesthetic of yours, but have not actually produced some project that puts them into practice.

Your own code will always be easier for you to understand, precisely because you wrote it. Do not mistake that phenomenon for a refactoring that actually solves real-world, pragmatic problems that cause bugs in a large piece of software.

I am sorry if I sound snippy but I am a little tired of the holier-than-thou attitude that you have been projecting with respect to how things "should" be done. You keep talking about how you want everything to be this-or-that, etc., criticizing others for sloppy coding standards, poor software design, and yet you seem unwilling to actually go do this stuff yourself. So you say that the project would never get done if you go did it the way you would have wanted? Well, maybe there's a lesson there.

I would very much welcome and even encourage you to prove me wrong, and I say that sincerely. But in the meantime, there's been enough talking.
David Haley aka Ksilyan
Head Programmer,
Legends of the Darkstone

http://david.the-haleys.org
[Go to top] top
Posted by Twisol   USA  (2,229 posts)  [Biography] bio
Date Mon 20 Sep 2010 12:52 AM (UTC)  [ quote  ]

Amended on Mon 20 Sep 2010 12:59 AM (UTC) by Twisol

Message
David, didn't you say you haven't actually worked with the MUSHclient source? Advice is wonderful, but yours feels really out of context here. I -understand- the code fine these days; that doesn't mean it's easy to understand. That also doesn't mean we won't make silly mistakes that could have been prevented with encapsulation and easing out some of the coupling.

[EDIT]: Rewriting it the way I would have written it would have two end results: (1) It would break compatibility. (2) It would never get done.
'Soludra' on Achaea

Blog: http://jonathan.com/
GitHub: http://github.com/Twisol
[Go to top] top
Posted by David Haley   USA  (3,881 posts)  [Biography] bio   Moderator
Date Mon 20 Sep 2010 12:05 AM (UTC)  [ quote  ]
Message
One can gain understanding of code without rewriting it to be how one would have written it.
David Haley aka Ksilyan
Head Programmer,
Legends of the Darkstone

http://david.the-haleys.org
[Go to top] top
Posted by Twisol   USA  (2,229 posts)  [Biography] bio
Date Sun 19 Sep 2010 06:55 PM (UTC)  [ quote  ]
Message
Worstje said:
Twisol, you have a habit of getting carried away in technical perfection. Hell, I have that a bit myself but have in the past few years been learning to deal with it a bit (or so I hope.) Since the deviation of such an utopian perfection is repeatedly brought up in near-circular arguments, I can see why Nick is defensive.

Mmm... You're probably right. It's just hard to understand the code in its current incarnation. The majority of my non-fix changes just aim to - incrementally - fix that.

Thank you for being a calm neutral observer, hehe.
'Soludra' on Achaea

Blog: http://jonathan.com/
GitHub: http://github.com/Twisol
[Go to top] top
Posted by Worstje   Netherlands  (867 posts)  [Biography] bio
Date Sun 19 Sep 2010 06:52 PM (UTC)  [ quote  ]
Message
I think I see why both sides are getting annoyed.

Twisol, you have a habit of getting carried away in technical perfection. Hell, I have that a bit myself but have in the past few years been learning to deal with it a bit (or so I hope.) Since the deviation of such an utopian perfection is repeatedly brought up in near-circular arguments, I can see why Nick is defensive.

Nick, I also see why Twisol tends to get a bit frustrated when conversing technical details with you. Given the fact MUSHclient is your baby, you are very protective of it, and with good right. But you are so protective of it that it is hard to actually contribute something worth mentioning, since anything other than trivial fixes are quickly turned into a 'refactoring might introduce bugs' discussion which leads them not being introduced.

Now, I am not going to agree with either side here. Refactoring might be good, it might be bad - it depends on the situation. The source might not be good, or you could say it is good enough. Both sides have good points to make.

In the end, let's just be mindful of eachother, and nip this war that's brooding inbetween the paragraphs in the bud, shall we?
[Go to top] top
Posted by Twisol   USA  (2,229 posts)  [Biography] bio
Date Sun 19 Sep 2010 06:21 PM (UTC)  [ quote  ]

Amended on Sun 19 Sep 2010 06:22 PM (UTC) by Twisol

Message
Nick Gammon said:
You said above that you have 25 warnings. Does the commit(s) fix those warnings? Or are you just making code cleanups that you think would be nice?

I spotted a few things while browsing the code, and they were easy to fix. I knew, for example, that MUSHclient had been free for quite a while now, so I expected that the registration code and order URLs were no longer used. I was correct.

Nick Gammon said:
Have you tested the revamped code (the new plugins interface?).

Informally, yes: all of the plugins I normally use still work just fine. Sorry I didn't mention it.

Nick Gammon said:
And can we please try to keep the discussion here? Rather than at Github? I am currently getting private emails, notes on Github, and postings here. It is hard to keep track of who thinks what, and in what order they thought it, if you start using all these different media to state your ideas.

I'm using the GitHub pull requests to discuss and review actual hard changes that are ready to go. It's closer to the material at hand, because you can click a tab and it'll show you the diff and affected files in a snap.

I use GitHub commit notes to point out issues with specific commits for a similar reason, except you don't even need to click to see what I'm talking about.

I discuss just about everything else here.

Like Worstje said, you can click the number next to your account name to see which notifications you've received. The news feed shown on your GitHub dashboard also collects things you're interested in, like changes to repositories you're watching.

The e-mail alerts from GitHub might be confusing, but I just use them as "Something happened on GitHub" notifications, and head to GitHub.com to dig into the details.

Nick Gammon said:
Twisol said:

Buffer overflows are an infamous source of security holes.


Yes, I'm aware of that. And if you look at my code you'll see that I rarely if ever just blindly use strcpy without checking what I am doing.

Yes, I know, on both counts. But are you really saying you never made a mistake? You yourself just fixed some bugs you both (a) introduced recently and (b) introduced a long time ago. I have every confidence in you as a programmer, Nick - especially considering that you've done much more than I ever have - but that doesn't mean I won't look at the code and ensure that things are absolutely solid.

Nick Gammon said:
If you want to go through and eliminate every warning, fix up every possible thing that lint might throw up, then I think you are better of writing a new client from scratch to much stricter coding standards.

Lint is a very different thing from standard compiler warnings, and this one is level 1. Unless warnings are disabled completely - is that even possible? - you'll always see this warning. For that matter, even using a #pragma I can't disable this warning under Release mode, which is quite bizarre. Clearly MS saw this as a very important issue, though obviously throwing an error would be taking it too far.

Nick Gammon said:
I would prefer that if you are going to go through the code with a fine-tooth comb, you find things that are actually potential problems for the end-user.

I am, frankly, confused. Are you the only one allowed to do refactorings of any sort? It would help me so much to know exactly what I am allowed to do in the source.

Nick Gammon said:
Now that was a genuine bug. But all these warning levels didn't find that.

I'm sorry Nick, but I really think you're missing the point.
'Soludra' on Achaea

Blog: http://jonathan.com/
GitHub: http://github.com/Twisol
[Go to top] top
Posted by Worstje   Netherlands  (867 posts)  [Biography] bio
Date Sun 19 Sep 2010 11:53 AM (UTC)  [ quote  ]
Message
Nick Gammon said:

And can we please try to keep the discussion here? Rather than at Github? I am currently getting private emails, notes on Github, and postings here. It is hard to keep track of who thinks what, and in what order they thought it, if you start using all these different media to state your ideas.


I try to keep things here, but if I see a pull request on Github, I feel it is a bit stupid to create a new topic and drag the conversation here, since at Github you can exactly point out what lines you are talking about and in what context, while such a paper trail would be hard to follow here. Although I do admit Github tends to be a bit spammy - but the Notification Center in the Account Settings can help to lower the spam significantly.
[Go to top] top
Posted by Nick Gammon   Australia  (18,770 posts)  [Biography] bio   Forum Administrator
Date Sun 19 Sep 2010 11:03 AM (UTC)  [ quote  ]

Amended on Sun 19 Sep 2010 11:09 AM (UTC) by Nick Gammon

Message
And can we please try to keep the discussion here? Rather than at Github? I am currently getting private emails, notes on Github, and postings here. It is hard to keep track of who thinks what, and in what order they thought it, if you start using all these different media to state your ideas.

Twisol said:

Buffer overflows are an infamous source of security holes.


Yes, I'm aware of that. And if you look at my code you'll see that I rarely if ever just blindly use strcpy without checking what I am doing.

Example 1:


pData->m_strFilename = new char [strlen (sName) + 1];
strcpy (pData->m_strFilename, sName);


I allocated memory for the string, and thus the strcpy can't overflow.

Example 2:


static char sPathName [_MAX_PATH];

  // ensure not too long
  strFileName = strFileName.Left (sizeof (sPathName) - 1);
  
  // copy to retain
  strcpy (sPathName, (const char *) strFileName);


I truncate the string to fit into the place the strcpy is going to.

Example 3:


    strcpy (filedlg.m_ofn.lpstrFile, "");


The destination field will hold an empty string.

If you want to go through and eliminate every warning, fix up every possible thing that lint might throw up, then I think you are better of writing a new client from scratch to much stricter coding standards.

I would prefer that if you are going to go through the code with a fine-tooth comb, you find things that are actually potential problems for the end-user.

For example, I found a place where I set a boolean along these lines:


m_bPluginProcessesOpenTag = m_CurrentPlugin->m_PluginCallbacks [ON_PLUGIN_MXP_OPENTAG].isvalid ();


Unfortunately, if the first plugin handles MXP open tags (which sets this flag) and the second plugin doesn't handle them, the flag is cleared, and then the first plugin didn't get the callback.

It should have read:


if (m_CurrentPlugin->m_PluginCallbacks [ON_PLUGIN_MXP_OPENTAG].isvalid ())
  m_bPluginProcessesOpenTag = true;


That way the flag is cumulative. Now that was a genuine bug. But all these warning levels didn't find that.
- Nick Gammon

www.gammon.com.au, www.mushclient.com
[Go to top] top
Posted by Nick Gammon   Australia  (18,770 posts)  [Biography] bio   Forum Administrator
Date Sun 19 Sep 2010 10:48 AM (UTC)  [ quote  ]
Message
Twisol, when you send me pull requests can you please be a bit more specific about what they are addressing?

http://github.com/nickgammon/mushclient/pull/8

Your comments on the pull say what you did (well the code pretty much says the same thing) but why you did it is less clear.

You said above that you have 25 warnings. Does the commit(s) fix those warnings? Or are you just making code cleanups that you think would be nice?

Have you tested the revamped code (the new plugins interface?).

I would rather you be testing that your existing plugins still work, than go through finding the occasional variable that might not be referenced. One helps keep the client stable, the other saves a couple of bytes.
- Nick Gammon

www.gammon.com.au, www.mushclient.com
[Go to top] top
Posted by Nick Gammon   Australia  (18,770 posts)  [Biography] bio   Forum Administrator
Date Sun 19 Sep 2010 01:04 AM (UTC)  [ quote  ]
Message
I've now revamped the plugins list to change it from an MFC-style list to an STL one. This is mainly a stylistic change, but it helps move MUSHclient towards using STL containers.
- Nick Gammon

www.gammon.com.au, www.mushclient.com
[Go to top] top
Posted by Twisol   USA  (2,229 posts)  [Biography] bio
Date Sat 18 Sep 2010 06:52 AM (UTC)  [ quote  ]

Amended on Sat 18 Sep 2010 06:57 AM (UTC) by Twisol

Message
Your current master compiles cleanly with only 193 warnings ([EDIT]: using /W3). If I add a compiler switch /D "_CRT_SECURE_NO_WARNINGS" it cuts them down to 25. (It complains that some of the functions being used don't take a buffer length, so they're susceptible to overflows.)

Going to dig into the source soon.
'Soludra' on Achaea

Blog: http://jonathan.com/
GitHub: http://github.com/Twisol
[Go to top] top
Posted by Nick Gammon   Australia  (18,770 posts)  [Biography] bio   Forum Administrator
Date Sat 18 Sep 2010 06:32 AM (UTC)  [ quote  ]
Message
Let's move on and test the changes I have made already. This is starting to sound like the refactoring argument again. I have said, time and time again, I know the code isn't perfect, and you can always come back and say "oh dear, I found a problem here on line 7654 of file xxx.cpp". I know there are problems.

I don't want to feel that whenever I make x changes, you will come back and want x+1 changes, because that process never ends, obviously.
- Nick Gammon

www.gammon.com.au, www.mushclient.com
[Go to top] top

The dates and times for posts above are shown in Universal Co-ordinated Time (UTC).

To show them in your local time you can join the forum, and then set the 'time correction' field in your profile to the number of hours difference between your location and UTC time.

6,162 views.

This is page 2, subject is 3 pages long:  [Previous page]  1  2 3  [Next page]

[Reply to this subject]  Reply to this subject   [New subject]  Start a new subject   [Refresh] Refresh page

Go to topic:           Search the forum

[Go to top] top

[Home]

Written by Nick Gammon - 5K

Comments to: Gammon Software support
[RH click to get RSS URL] Forum RSS feed ( http://www.gammon.com.au/rss/forum.xml )

[Best viewed with any browser - 2K]    [Internet Contents Rating Association (ICRA) - 2K]    [Web site powered by FutureQuest.Net]