|
You could reduce the number of buffers you are using, as that seems to complicate your code. You are also trying to return FullMessage which is a temporary buffer inside the function. So as soon as you return from the function that buffer's memory space is free to be overwritten. And what is that exit(1) call there for? No wonder you never return from your function. You need something simple like (using C style only):
#define MAX_BUFFER 100
char * CLASS_LCM1602::BuildMessage(char *title, int value)
{
char* buffer = (char*)malloc(MAX_BUFFER );
snprintf(buffer, MAX_BUFFER , "%s %d", title, value);
return buffer;
}
And then the call is:
char *message = i2c.BuildMessage(title, value);
printf("%s\n", message);
free(message);
modified 17-Dec-18 5:03am.
|
|
|
|
|
Quote: snprintf(buffer, sizeof(buffer), "%s %d", title, value); // this assumes 100 characters is enough
That should be
snprintf(buffer, 100, "%s %d", title, value);
|
|
|
|
|
|
Thanks,
so I goofed by not allocating space / format for the "concatenated " message.
Since this is just basic "title value " function the new buffer should not be fixed / defined. I am planning to pass more values with same title later.
But it is a good practice not to allocate more than necessary, right?
Thanks again.
|
|
|
|
|
Yes, otherwise you tend to duplicate effort and waste space. In fact my code relies on the total generated string being less than 100 characters, and could fail for longer titles. A better way of doing it would be to measure the fields that are to be entered to ensure you have enough space. Something like:
char * CLASS_LCM1602::BuildMessage(char *title, int value)
{
int bufferSize = strlen(title) + 1 + strlen("-2147483647") + 1; char* buffer = (char*)malloc(bufferSize);
snprintf(buffer, bufferSize, "%s %d", title, value);
return buffer;
}
|
|
|
|
|
Personally code in this form I would not allow in our code base as it will lead to bleeding memory when used by others.
It is not clear that the function allocates memory, to anyone other than yourself and not being aware the function setups to use the char* pointer return and lose it ... take this basic code
CLASS_LCM1602 myCLASS_LCM1602 = .. some initialization;
/* now use your function */
std::cout << myCLASS_LCM1602.BuildMessage("Demo my Title", 10) << "\n";
That bleeds memory and the whole setup of the function encourages it because people will be tempted to feed the return char* value into functions.
For my 2 cents I would force you to use this interface for what you wanted to do .. Joe below has made the same comment.
int CLASS_LCM1602::BuildMessage(const char *title, int value, char* buffer, int bufSize)
It is much cleaner in that they pass you a buffer to fill and it's size and you return how many characters you put in the buffer.
Both you and they can be safe about handling the string becasue all sizes are known both directions.
If they want to allocate a buffer they can but they then know they have responsibility to dispose it.
It is also more flexible because you can use it on temporary stack allocated buffers .. something like this is a perfectly valid temp buffer with no allocate required.
char tempBuf[256];
As an example the actual Windows framework MFC is copying does exactly that 
GetWindowTextA function | Microsoft Docs[^]
In vino veritas
modified 18-Dec-18 0:59am.
|
|
|
|
|
Note that title is a static string, not a buffer; you cannot safely append to it.
Your prototype should be more like:
BuildMessage(const char* pTitle, int value, char* pDstBuffer, size_t dstBufferLen); Also note that you are doing an snprintf, but aren't using strncat.
|
|
|
|
|
When I started I actually had two function steps - convert value int to char* then strcat the title and value.
It did not work so I inserted strcpy, but forgot the build resulting buffer.
snprintf was just to do conversion.
Now it does it all in few lines of code.
Thanks to the forum. Appreciate it.
|
|
|
|
|
I'm cleaning up a chess program of mine, which includes a clock feature. For those unfamiliar with chess clocks, both players start the game with a certain amount of time, and each of their clocks counts down when and only when it's currently their turn. The program has GUI, and the displays for the timers have to be prompted to redraw every time a second flips.
I had previously implemented this by having a timer thread that stores the time when a turn starts, then constantly re-checks the time in a loop, using this to update the active player's time on every iteration. This worked like a charm, but I understand that spinning like that is bad practice for wasting cycles, so I tried redoing it using wait_until(), roughly like this:
HWND g_mainWindowHandle;
std::condition_variable g_timerConditional;
std::mutex g_timerLock;
std::atomic<bool>g_turnIsOver;
std::atomic<std::chrono::nanoseconds>g_blackPlayerNanosecondsLeft;
std::atomic<std::chrono::nanoseconds>g_whitePlayerNanosecondsLeft;
void turnTimer(std::atomic<std::chrono::nanosecondsNanosecondsLeft>*activePlayerNanosecondsLeft,
Rect*displayRect)
{
std::chrono::nanoseconds currentNanosecondsLeft{ activePlayerNanosecondsLeft->load() };
std::chrono::nanoseconds nanosecondsToNextSecond{ currentNanosecondsLeft % 1000000000 };
std::chrono::steady_clock::time_point nextSecondBoundary{
std::chrono::steady_clock::now() + nanosecondsToNextSecond };
std::unique_lock<std::mutex>lock(g_timerLock);
std::cv_status status{
g_timerConditional.std::condition_variable::wait_until(lock, nextSecondBoundary) };
while (!g_turnIsOver.load())
{
if (status == std::cv_status::timeout)
{
currentNanosecondsLeft -= nanosecondsToNextSecond;
*activePlayerNanosecondsLeft = currentNanosecondsLeft;
nanosecondsToNextSecond = std::chrono::nanoseconds(1000000000);
nextSecondBoundary += nanosecondsToNextSecond;
InvalidateRect(g_mainWindowHandle, displayRect, FALSE);
}
status = g_timerConditional.std::condition_variable::wait_until(lock, nextSecondBoundary);
}
currentNanosecondsLeft -=
std::chrono::steady_clock::now() - (nextSecondBoundary - nanosecondsToNextSecond);
*activePlayerNanosecondsLeft = currentNanosecondsLeft;
g_turnIsOver = false;
InvalidateRect(g_mainWindowHandle, displayRect, FALSE);
}
Each time the main thread moves a piece, it sets g_turnIsOver to true, calls g_timerConditional.notify_one(), calls join() on the current timer thread, and starts a new timer thread for the new active player. While this approach seems to work well for the majority of moves, for the rest there is a generous fraction of a second of lag between clicking to make a move and seeing the board update. I'm quite confident the timer code is the culprit in some capacity. Is this likely to be an example of what the reference for wait_until() means when it says "the function also may wait for longer than until after timeout_time has been reached due to scheduling or resource contention delays"? Is there a way to fix it, or will the entire approach of using wait_until() not work?
modified 16-Dec-18 10:48am.
|
|
|
|
|
I would use a different approach:
- Have an event queue (practically - a single element) that contains pairs - <player, time="" of="" move="">
- When a player makes a move, insert a <player, time="" of="" move=""> pair into the queue
- Have a timer thread that updates the clock display every <tbd> milliseconds
- Before updating the clock, it:
4.1 Reads the current time using the same clock used by the player threads
4.2 Checks the event queue to see if a move was made, and when it was made
4.3 If a move was made, it updates the players' times, stops one clock, and starts the other
There will always be an unavoidable lag in the visual display of the clock because of the O/S's scheduling requirements, but the internal counts will be accurate.
Freedom is the freedom to say that two plus two make four. If that is granted, all else follows.
-- 6079 Smith W.
|
|
|
|
|
Having a timer activate every millisecond involves using something like sleep_for, right? Is that less susceptible to delays than wait_until is? Either way, it does certainly having the advantage that any timer-related delays won't block the main thread.
|
|
|
|
|
Hi,
I doubt your condition_variable::wait_until code is causing the redraw delay based on what you've shown.
Invalidating the rect simply creates an update region and then marks an internal 'dirty bit' associated with the window. The operating system is somewhat lazy updating the window... it could be redrawn a few milliseconds later... or several seconds later depending on how many messages are in the message queue.
If you want an immediate redraw add a call to the UpdateWindow function immediately following your invalidation.
Best Wishes,
-David Delaune
|
|
|
|
|
While I must admit that I don't know how to gracefully debug the timer, I'm pretty sure the join call is producing at least some of the noticeable delays. I tried just adding an UpdateWindow call after the InvalidateRect call in turnTimer as a simple test, but that deadlocked the program, which led me to read about how it's flatly incorrect to do a lot of kinds of window manipulations from another thread. I'm not sure if InvalidateRect is one of them and I just kept getting lucky with the original implementation, which also called it from the timer thread, but I suppose the fact that I'm not sure is enough reason to switch to something like Daniel described even if I weren't getting the lags. I'll try it and see what happens.
modified 16-Dec-18 22:25pm.
|
|
|
|
|
Hi,
Yep, Daniel gave you some great advice. You don't really need a second thread.
My assessment is the same... InvalidateRect does not redraw the window, it simply marks it as dirty. Your main source of visual delay is most likely waiting for a WM_PAINT message.
If you use the event driven model Daniel suggested maybe you could add a redraw event handler that invalidates and redraws the window.
Best Wishes,
-David Delaune
|
|
|
|
|
I've implemented this approach, and it's working for me. It proved to be easier to factor for multi-platform support than what I had before would have been, too.
|
|
|
|
|
I'm happy to have helped.
Freedom is the freedom to say that two plus two make four. If that is granted, all else follows.
-- 6079 Smith W.
|
|
|
|
|
I'm looking for a way for my application to receive a notification whenever the PC connects to a WiFi network. I need to know the name of the network so that I can configure stuff differently depending upon which network the user is connected to.
I found this on MSDN: AddConnectNotify function | Microsoft Docs[^]
However, it looks like that function is for receiving notification of connections to network resources on the network that you're already connected to. It is not for receiving a notification of actually connecting to the network itself.
Does anyone know what I'm looking for?
Thank you.
SOLUTION:
I found this function which is exactly what I'm looking for: WlanRegisterNotification function | Microsoft Docs[^]
The difficult we do right away...
...the impossible takes slightly longer.
modified 14-Dec-18 22:29pm.
|
|
|
|
|
Hi,
Program is throwing exceptions "This command is not available" while calling "Editpaste" and "Editreplace" functions from code in Word automation. This is happening sporadically not every time. Any suggestions please?
|
|
|
|
|
narasingubhanu wrote: Any suggestions please?
Those commands are not working correctly. Exactly why is a mystery, as we have no idea what your code is doing.
|
|
|
|
|
Yes. its mystery on Windows 10 only . My code will edit word document with some values and it gives to printer.
|
|
|
|
|
I hope this is not going to be another one of "read the book" discussion.
( And if you already do not like my remark, do not read the rest of this post)
This is very simple , but as always I am not sure about using correct terminology.
The attached code , struct variable , is taken from C code tutorial.
gcc++ compiler generates this error
sorry, unimplemented: non-trivial designated initializers not supported
Mrs Google say it is because gcc++ does not like C style "reference" "." It also say not to use "=" , but use ":" .
OK, I generally can follow instructions , BUT
If I do not use "." my IDE won't supply all available "options" anymore.
No big deal, in this case, however I like to know if I can change some options in gcc++ or if this a IDE "issue".
struct spi_ioc_transfer tr = {
tx_buf:(unsigned long)tx,
this works fine no error
};
|
|
|
|
|
You have either set the C++ standard flags low or the default on your compiler is not c11++ or greater
I have no issue with GCC or VS2017 taking structs like that.
try the flag
-std=c++11
or I would probably try and see if it is available
-std=c++20
BTW if it is actually a C file you need different flag .. I know you were playing with C files before
-std=c11
Now that I think about that I am guessing that is the issue you are compiling C files with G++ .. don't 
Setup your make file to send C files to the c compiler gcc, you can make a rule based on the extension .c vs .cpp
.c files => gcc
.cpp files => g++
Visual Studio actually does exactly that it uses the filename extension to decide on c vs c++ compiling, which is why it works on it.
In vino veritas
modified 12-Dec-18 12:51pm.
|
|
|
|
|
Is there any method to list files from local HDD into a CListView, but, most important, have two points:
1. Have an item icon just like you see in standard windows explorer.
2. Have right click menu just like you have in standard windows explorer.
modified 11-Dec-18 7:42am.
|
|
|
|
|
|
No, there is no problem listing files found on HDD, but setup them an icon (in CListView of course) just like Windows Explorer does, and then have the same contextual menu like Windows Explorer has ...
|
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
