See my updated attachment.
In this version I did not try to do cleanups, instead I
declare errorStr as local variable.
As for "check", I find that it gets redefined in
graph/viewAlone/viewAlone.h, graph/viewman/viewman.h:
#define check(code) checker(code,__LINE__,"")
Also I don't like its overhead when we only read a few bytes,
I hope in the future we can avoid the overhead if we are
building in release mode (aka when there is no DEBUG defined).
- Qian
On 5/26/24 20:05, Waldek Hebisch wrote:
On Sat, May 25, 2024 at 06:27:10PM +0800, Qian Yun wrote:
Sorry, the problem is not "fricas_sprintf_to_buf", but
"extern char errorStr [80];".
I want to avoid this global variable and implementation
detail to leak to util.c.
So you are trying to clean up the code. I must say that
I would prefer to have 'checked' read that would loop in
case of short read (usually 'read' delivers all requested
data, but it can return only part of what was requested
in in such case we should loop (this is not handled in
current code)). Such 'checked' read would get parameters
needed to build error message (so no global buffer and no
'sprintf' at call site).
--
You received this message because you are subscribed to the Google Groups "FriCAS -
computer algebra system" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/fricas-devel/f2e8714f-8d3a-44ac-aa7a-51ee4750209a%40gmail.com.
diff --git a/src/graph/include/spadAction2d.H1 b/src/graph/include/spadAction2d.H1
index 455fef4a..fd4457ad 100644
--- a/src/graph/include/spadAction2d.H1
+++ b/src/graph/include/spadAction2d.H1
@@ -1,2 +1 @@
-extern int readViewman(void *, int);
extern int spadAction(void);
diff --git a/src/graph/include/spadAction3d.H1 b/src/graph/include/spadAction3d.H1
index cc2cc432..753346ae 100644
--- a/src/graph/include/spadAction3d.H1
+++ b/src/graph/include/spadAction3d.H1
@@ -1,3 +1,2 @@
-extern int readViewman(void *, int);
extern void scalePoint(viewTriple *);
extern int spadAction(void);
diff --git a/src/graph/view2D/spadAction2d.c b/src/graph/view2D/spadAction2d.c
index a923ad88..c5d71714 100644
--- a/src/graph/view2D/spadAction2d.c
+++ b/src/graph/view2D/spadAction2d.c
@@ -44,22 +44,6 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
#include "util.H1"
#include "strutil.h"
-/******************************
- * int readViewman(info,size) *
- ******************************/
-
-int
-readViewman(void * info,int size)
-{
- int mold = 0;
-
- fricas_sprintf_to_buf3(errorStr, "%s %d %s", "read of ", size,
- " bytes from viewport manager\n");
- mold = check(read(0,info,size));
- return(mold);
-
-}
-
/********************
* int spadAction() *
********************/
@@ -95,8 +79,7 @@ spadAction(void)
case changeTitle:
readViewman(&i1,intSize);
- readViewman(viewport->title,i1);
- viewport->title[i1] = '\0';
+ readViewmanStr(viewport->title, i1, sizeof(viewport->title));
writeTitle();
writeControlTitle();
XFlush(dsply);
@@ -105,9 +88,7 @@ spadAction(void)
case writeView:
readViewman(&i1,intSize);
- readViewman(filename,i1);
- filename[i1] = '\0';
- fricas_sprintf_to_buf1(errorStr, "%s", "writing of viewport data");
+ readViewmanStr(filename, i1, sizeof(filename));
i3 = 0;
readViewman(&i2,intSize);
while (i2) {
diff --git a/src/graph/view2D/viewport2D.c b/src/graph/view2D/viewport2D.c
index 95b15f21..bc428ce5 100644
--- a/src/graph/view2D/viewport2D.c
+++ b/src/graph/view2D/viewport2D.c
@@ -514,7 +514,7 @@ makeViewport(char *title,int vX,int vY,int vW,int vH,int showCP)
fprintf(stderr,"view2D: Made a viewport\n");
#endif
- strcpy(viewport->title,title);
+ strncpy(viewport->title, title, sizeof(viewport->title) - 1);
viewport->closing = no;
viewport->allowDraw = yes; /* just draw axes the first time around */
diff --git a/src/graph/view3D/header.h b/src/graph/view3D/header.h
index 21009335..622b744e 100644
--- a/src/graph/view3D/header.h
+++ b/src/graph/view3D/header.h
@@ -269,7 +269,7 @@ typedef struct _buttonStruct {
typedef struct _controlPanelStruct {
Window controlWindow, messageWindow, colormapWindow;
- char message[40];
+ char message[80];
buttonStruct buttonQueue[maxButtons3D];
} controlPanelStruct;
diff --git a/src/graph/view3D/spadAction3d.c b/src/graph/view3D/spadAction3d.c
index 1cf79001..4f1c1313 100644
--- a/src/graph/view3D/spadAction3d.c
+++ b/src/graph/view3D/spadAction3d.c
@@ -46,17 +46,6 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
#include "all_3d.H1"
#include "strutil.h"
-int
-readViewman (void *info,int size)
-{
- int m = 0;
-
- fricas_sprintf_to_buf1(errorStr, "%s", "read from viewport manager\n");
- m = check(read( 0, info, size));
-
- return(m);
-
-}
void
scalePoint (viewTriple *p)
{
@@ -398,8 +387,7 @@ spadAction (void)
case changeTitle:
readViewman(&i1,intSize);
- readViewman(viewport->title,i1);
- viewport->title[i1] = '\0';
+ readViewmanStr(viewport->title, i1, sizeof(viewport->title));
writeTitle();
switch (doingPanel) {
case CONTROLpanel:
@@ -418,9 +406,7 @@ spadAction (void)
case writeView:
readViewman(&i1,intSize);
- readViewman(filename,i1);
- filename[i1] = '\0';
- fricas_sprintf_to_buf1(errorStr, "%s", "writing of viewport data");
+ readViewmanStr(filename, i1, sizeof(filename));
i3 = 0;
readViewman(&i2,intSize);
while (i2) {
diff --git a/src/graph/view3D/viewport3d.c b/src/graph/view3D/viewport3d.c
index 0e6530b9..b50375c5 100644
--- a/src/graph/view3D/viewport3d.c
+++ b/src/graph/view3D/viewport3d.c
@@ -545,7 +545,7 @@ makeViewport (void)
viewport->thetaObj = 0.0;
viewport->phiObj = 0.0;
- strcpy(viewport->title,viewData.title);
+ strncpy(viewport->title, viewData.title, sizeof(viewport->title) - 1);
viewport->axesOn = yes;
viewport->regionOn = no;
diff --git a/src/graph/viewman/fun2D.c b/src/graph/viewman/fun2D.c
index 666fd0e8..b61e24bb 100644
--- a/src/graph/viewman/fun2D.c
+++ b/src/graph/viewman/fun2D.c
@@ -137,6 +137,7 @@ funView2D(int viewCommand)
i1 = strlen(s1);
code = write(viewport->viewOut,&i1,intSize);
code = write(viewport->viewOut,s1,i1);
+ free(s1);
break;
case writeView:
@@ -151,6 +152,7 @@ funView2D(int viewCommand)
i2 = get_int(spadSock);
code = write(viewport->viewOut,&i2,intSize);
}
+ free(s1);
break;
case spadPressedAButton:
diff --git a/src/graph/viewman/fun3D.c b/src/graph/viewman/fun3D.c
index fd4e524e..64e59f2c 100644
--- a/src/graph/viewman/fun3D.c
+++ b/src/graph/viewman/fun3D.c
@@ -217,6 +217,7 @@ funView3D(int viewCommand)
i1 = strlen(s1);
code = write(viewport->viewOut,&i1,intSize);
code = write(viewport->viewOut,s1,i1);
+ free(s1);
break;
case writeView:
@@ -231,6 +232,7 @@ funView3D(int viewCommand)
i2 = get_int(spadSock);
code = write(viewport->viewOut,&i2,intSize);
}
+ free(s1);
break;
case diagOnOff:
diff --git a/src/include/util.H1 b/src/include/util.H1
index 0adcdc3b..f86a0e0f 100644
--- a/src/include/util.H1
+++ b/src/include/util.H1
@@ -4,3 +4,5 @@ extern char * saymemWithLine(char *, int, int, int);
extern void myfree(void *, int);
extern XPoint getWindowPositionXY(Display *, Window);
extern XPoint getWindowSizeXY(Display *, Window);
+extern int readViewman(void *, int);
+extern int readViewmanStr(char *, int, int);
diff --git a/src/lib/util.c b/src/lib/util.c
index ad468f8d..1cc28858 100644
--- a/src/lib/util.c
+++ b/src/lib/util.c
@@ -41,6 +41,7 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
#include <X11/Xlib.h>
#include <X11/Xutil.h>
#include "view.h"
+#include "strutil.h"
#include "util.H1"
@@ -150,3 +151,41 @@ getWindowSizeXY(Display *display,Window w)
return (size);
}
+
+int
+readViewman(void *info, int size)
+{
+ int m = 0;
+ char errorStr[80];
+
+ fricas_sprintf_to_buf3(errorStr, "%s %d %s", "read of ", size,
+ " bytes from viewport manager\n");
+ m = check(read(0, info, size));
+
+ return(m);
+}
+
+/*
+ Read 'size' bytes from stdin, and store it into 'buf' with size 'buf_size'.
+ If 'buf' is not large enough to hold the string, the result is truncated.
+*/
+
+int
+readViewmanStr(char *buf, int size, int buf_size)
+{
+ int m = 0;
+ char errorStr[80];
+
+ fricas_sprintf_to_buf3(errorStr, "%s %d %s", "read of ", size,
+ " bytes from viewport manager\n");
+ if (size < buf_size) {
+ m = check(read(0, buf, size));
+ } else {
+ char *tmp = malloc(size * sizeof(char));
+ m = check(read(0, tmp, size));
+ strncpy(buf, tmp, buf_size - 1);
+ free(tmp);
+ }
+
+ return(m);
+}
